-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doing .text() of a <script> brings empty string on 1.0.0-rc.2 #1050
Comments
Thanks for the report! This is a tricky problem. The updated behavior was merged to
Interestingly, this is not observable from Chrome or Firefox--the method returns the text contents of From this perspective, the change is technically valid. To promote portability between Cheerio and jQuery and between all web browsers, Cheerio should similarly recommend that consumers use That said, such a policy definitely feels like a lowest common denominator solution, one that accounts for web browsers that are over eight years old and have negligible market share. It's also important to note that jQuery itself no longer supports Internet Explorer releases prior to version 9. (That detail tends to suggest a jQuery documentation change. I've opened an issue to discuss this with the jQuery So with all of that said, I'm leaning towards reverting gh-1018. What do you think, @fb55? |
This is an interesting case. I was assuming that |
A bit bias ;) but the current behavior does support the specifications in the jQuery docs as you mentioned. Having both options is best? Users wanting to scrape script tags can use .html() and users wanting to ignore script tags can use .text(). Reverting #1018 will give users only one option. |
I spent a lot of time wondering why I wasn't able to get the I agree with @fb55, I was confused as both I'm okay using |
I was also confused, that the |
Honestly I think I just pulled out every strand of hair on my head head while trying to figure why I kept getting an empty string! |
Let's revert #1018, the current behavior seems pretty surprising. |
Before publishing that fix, please see if enzyme's test suite passes with it :-) |
@ljharb Do you have a suspicion that things might break? Would love to hear another take on this 😄 |
It’s possible it’d only be conceptual and not actual breakage, but I’d hope enzyme has enough test coverage that it’d notice the change. |
Next steps:
|
@ljharb The last commit on the enzyme master branch is two years old, even though tags still seem to be pushed. What is the best source for enzyme's source code, so we can run the test suite? |
@fb55 thats the date the commit was authored; i landed it a week ago. The master branch is quite up to date. |
Ah, interesting 😄 Thanks for clarifying! |
I decided to punt on this for now. For reference, enzyme actually does not have a test covering it. |
@fb55 that is true; the second checkbox in the list above never happened. I can certainly try to add a test, or at least open a PR/branch with one, if that would help you fix this in cheerio? |
Yes. For all, please use html() instead of text() for now, it works! |
It won’t work if you need the text instead of the HTML. |
It only took five years, but this has been resolved. There are now |
Hi, I've been doing .text() of tags for scraping purposes, and the latest release candidate breaks this. jQuery 1~3 and cheerio ~0 both works fine.
The text was updated successfully, but these errors were encountered: