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

Drop IE 11 support by default #5090

Merged
merged 5 commits into from
Sep 25, 2018
Merged

Conversation

Timer
Copy link
Contributor

@Timer Timer commented Sep 25, 2018

This drops IE 11 support by default. We no longer ship or use a polyfill entrypoint.

Introduces new package, react-app-polyfill, with entry points for ie9, ie11, and jsdom.

This also goes a step further and polyfills some things previously not polyfilled, i.e.:

  1. Symbol (used by for...of)
  2. Array.from (used by [...arr])

TODO:

  • verify Array.from is needed to perform array spread and not handled magically by babel somehow
  • add docs on IE 11/IE 9 support


### Entry Points

There is no single entry point. You can only import individual browser support levels.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • "You can import the entry point for the minimal version you intend to support. For example, if you import the IE9 entry point, this will include IE10 and IE11 support."

#### Internet Explorer 9

```js
// # index.js
Copy link
Contributor

Choose a reason for hiding this comment

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

// # index.js will be confusing.

Maybe

// This must be the first line in src/index.js

// React 16+ relies on Map, Set, and requestAnimationFrame
require('core-js/es6/map');
require('core-js/es6/set');
require('raf').polyfill(global);
Copy link
Contributor

Choose a reason for hiding this comment

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

global? This ain't Node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

D'oh!

@@ -22,7 +22,7 @@ module.exports = (resolve, rootDir, isEjecting) => {
// in Jest configs. We need help from somebody with Windows to determine this.
const config = {
collectCoverageFrom: ['src/**/*.{js,jsx}'],
setupFiles: [resolve('config/polyfills.js')],
setupFiles: [resolve('config/jest/polyfills.js')],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not react-app-polyfill/jsdom directly

Copy link
Contributor Author

@Timer Timer Sep 25, 2018

Choose a reason for hiding this comment

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

Good point. Though, this might be a good default for people who eject so they have a location to easily add new polyfills required for tests.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And if we do switch this, does it need to be <rootDir>/node_modules/react-app-polyfill/jsdom?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. Though, this might be a good default for people who eject so they have a location to easily add new polyfills required for tests.

At this point they might as well change the config directly.

And if we do switch this, does it need to be /node_modules/react-app-polyfill/jsdom?

Check Jest docs? I would expect it to be smart enough to resolve it somehow.

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Would be nice to actually test that it works but good enough for beta

@Timer
Copy link
Contributor Author

Timer commented Sep 25, 2018

Kitchensink passed, at least. I'll test this in IE 11 on my Windows laptop. I don't think I can get access to something with IE 9, but I think IE 11 can emulate it.

@Timer Timer merged commit 5599eff into facebook:next Sep 25, 2018
@Timer Timer deleted the feature/drop-ie-11-support branch September 25, 2018 20:08
@Timer Timer mentioned this pull request Sep 25, 2018
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018
* Drop ie 11 support and move polyfills to a new package

* More useful directions for what entry point to use
facebook#5090 (comment)

* Clear up what file this polyfill goes in
facebook#5090 (comment)

* Polyfill `window`, not `global`

* Remove proxy polyfill file
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants