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

Warn on missing Set/Map polyfills #10356

Merged
merged 4 commits into from
Aug 2, 2017
Merged

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 2, 2017

Adding an early check with a URL currently pointing to a gist.
We’ll move this to docs as part of 16 release.

We could do a warning instead, which would save some bytes. I opted for invariant for consistency with what we do for rAFs. Changed this to be a warning per feedback.

I check for specific methods we use. There’s some earlier versions of Firefox that added support gradually.

@sebmarkbage
Copy link
Collaborator

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 2, 2017

See above:

We could do a warning instead, which would save some bytes. I opted for invariant for consistency with what we do for rAFs.

Wanna change this for rAF to also be a warning?

@gaearon gaearon changed the title Crash on missing Set/Map polyfills Warn on missing Set/Map polyfills Aug 2, 2017
@gaearon
Copy link
Collaborator Author

gaearon commented Aug 2, 2017

I changed Map/Set to be a warning but left requestAnimationFrame an invariant. Can change this to warning as well (not sure if you intended that).

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 2, 2017

Changed rAF check to also be a warning in last commit.

@@ -25,6 +25,21 @@ import type {Deadline} from 'ReactFiberReconciler';
var invariant = require('fbjs/lib/invariant');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now unused. rm to fix lint.

@gaearon gaearon merged commit 4e6cc2f into facebook:master Aug 2, 2017
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.

3 participants