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

react-dom development bundle uses Array.from #12540

Closed
wKovacs64 opened this issue Apr 4, 2018 · 15 comments
Closed

react-dom development bundle uses Array.from #12540

wKovacs64 opened this issue Apr 4, 2018 · 15 comments

Comments

@wKovacs64
Copy link

wKovacs64 commented Apr 4, 2018

Do you want to request a feature or report a bug?

Bug (?)

What is the current behavior?

Currently, the 16.3.x versions of react-dom ship react-dom.development.js containing usage of Array.from which is not available in certain React-supported browsers like IE11. It causes breakage if those particular code paths are taken in such a browser (I ran into it using <StrictMode> in IE11).

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

Reproduction (should see "Hello, world!" but IE11 will crash and show nothing):
https://codepen.io/anon/pen/zWJBrO?editors=0010

What is the expected behavior?

react-dom.development.js would not use Array.from or document that a polyfill is required.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

react and react-dom: 16.3.0-alpha.0 through 16.3.1
Internet Explorer 11

@aweary
Copy link
Contributor

aweary commented Apr 4, 2018

Thanks for the report @wKovacs64, sorry about the unexpected breakage.

@gaearon what do you want to do here, stop using Array.from or ask that users polyfill it? It's currently being used by ReactStrictModeWarnings to convert a Set in a few places like:

const sortedComponentNames = Array.from(componentNames)

This case would be easy enough to refactor without Array.from

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

We definitely should not have been using Array.from there. 😞

@bvaughn
Copy link
Contributor

bvaughn commented Apr 4, 2018

Sorry 'bout that folks~

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

thanks

@sophiebits
Copy link
Collaborator

Why shouldn't we have been using Array.from?

@bvaughn
Copy link
Contributor

bvaughn commented Apr 4, 2018

No IE support.

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

We used to run in IE11 with Map/Set/rAF polyfills so IMO adding dependencies on other builtins is a breaking change. We didn't ask people to polyfill the whole ES6 environment, just these three things.

@sophiebits
Copy link
Collaborator

OK. Maybe in the future. We shouldn't get in the habit of inlining implementations of ES6 builtins if we can avoid it because it bloats bundle size and is also less maintainable.

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

Agree! This one is DEV-only. Maybe we can require full ES6 env with 17.

@blling
Copy link

blling commented Apr 4, 2018

Sounds good! 👍

@danvc
Copy link

danvc commented Apr 4, 2018

@gaearon you mean, new implementations from 17 should be accepting ES6?

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

I'm saying that in React 17 we can bump the runtime requirement to assume ES6 environment.

@danvc
Copy link

danvc commented Apr 4, 2018

Uhm... gotcha. I was just concerned about many applications (from DoD) using React that still runs under IE11 that isn't compatible with many ES6 functions.

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

If we bump minimal requirement to ES6 this will only require polyfilling some functions. It doesn't necessarily mean dropping IE support.

@wKovacs64
Copy link
Author

Thanks all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants