Skip to content
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

Remove String.prototype.split polyfill warning #7629

Merged
merged 1 commit into from
Sep 1, 2016

Conversation

nhunzaker
Copy link
Contributor

String.prototype.split is a ubiquitous part of the ES3 spec. Tracking back (#1516), I can't see a specific reason to include it in the required polyfills.

@syranide: did I miss anything?

@nhunzaker nhunzaker changed the title Remove String.prototype.split polyfill wanring Remove String.prototype.split polyfill warning Sep 1, 2016
@chicoxyzzy
Copy link
Contributor

chicoxyzzy commented Sep 1, 2016

Also all other methods there are in ES5 and IE9 supports them all and React docs says

React supports most popular browsers, including Internet Explorer 9 and above.

https://facebook.github.io/react/docs/working-with-the-browser.html#browser-support

@syranide
Copy link
Contributor

syranide commented Sep 1, 2016

It's a DEV-only thing so I'm not sure if it really matters and yeah it's so old it would surprise me if you can even find a browser that lacks it today. So yeah it seems quite useless there, but then again, it's not hurting anyone either, so take my thumbs up and a shrug ;)

@aweary
Copy link
Contributor

aweary commented Sep 1, 2016

Might as well take this then, at the very least to avoid confusing any future contributors 👍

One thing I just noticed is that https://fb.me/react-warning-polyfills links to:

https://facebook.github.io/react/docs/working-with-the-browser.html#polyfills-needed-to-support-older-browsers

And the polyfill section it links to doesn't exist anymore. I think it should be updated to point to https://facebook.github.io/react/docs/working-with-the-browser.html#browser-support. @zpao can we get that link updated? Though honestly there isn't really a whole lot of useful details in that section, so maybe it's not worth linking to a docs page for this anymore?

@aweary aweary added this to the 15-next milestone Sep 1, 2016
@aweary aweary merged commit 5c47920 into facebook:master Sep 1, 2016
@zpao zpao modified the milestones: 15-next, 15.4.0 Oct 4, 2016
zpao pushed a commit that referenced this pull request Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants