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

Document the extent of ES6 polyfilling #969

Closed
jhorneman opened this issue Oct 26, 2016 · 14 comments
Closed

Document the extent of ES6 polyfilling #969

jhorneman opened this issue Oct 26, 2016 · 14 comments

Comments

@jhorneman
Copy link
Contributor

Assuming no major misunderstandings from my side: It was not clear me from the documentation that create-react-app installs fetch (whatwg-fetch), and that this is then available to me in my own JavaScript code. I only found this out when I looked at create-react-app (CRA)'s package.json file and tried it out.

I assume the presence of fetch and its use in user code is intended, but I cannot assume this is true for everything inside CRA's package.json file.

It seems to me there are three kinds of dependencies inside create-react-app:

  1. development dependencies, such as rimraf, which are needed for CRA's core functionality but not for any code running in the browser - the equivalent of installing with --save-development.
  2. dependencies that are implementation details and while they might be useful for my own code, are not guaranteed to stay in future versions of CRA.
  3. dependencies that I can use in my own code and that are guaranteed to stay, such as, presumably, fetch, until fetch is available natively in all major browsers.

It is hard to tell right now which dependency is which. It would be great to have this documented somewhere. I would propose something, but I lack the requisite knowledge.

@thien-do
Copy link
Contributor

Hi @jhorneman , let's keep track here #531

@jhorneman
Copy link
Contributor Author

Thanks. Sorry, I only looked through the docs issues, not the pull requests. I'll do that next time!

@gaearon
Copy link
Contributor

gaearon commented Oct 27, 2016

It's not meant to be used as a dependency, it's a polyfill. So you should not try to import it.

It just makes sure you have a global fetch() and global Promise whether or not browser implements them.

Wrong:

import fetch from 'whatwg-fetch'
fetch()

Right:

fetch()

I agree we should document available polyfills.

@gaearon gaearon changed the title Document available packages such as fetch Document the extent of ES6 polyfilling Nov 20, 2016
@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

People commonly mistakingly assume Create React App polyfills ES6 runtime methods like Array.from. We should document this.

@stujo
Copy link

stujo commented Nov 29, 2016

What is the best practice to adding polyfills to CRA apps?

For example: I want to polyfill Array.includes and I'm wondering where and how to add it and be consistent with the CRA worldview

@gaearon
Copy link
Contributor

gaearon commented Nov 29, 2016

If you're okay with using CDNs, consider using https://polyfill.io/. Don't forget to add Promise, fetch and Object.assign to excludes because they're already bundled with CRA.

If you'd prefer to bundle them with your app, you can install core-js and import corresponding polyfills from src/index.js before other imports. I don't recommend importing complete core-js because it is pretty heavy.

@stujo
Copy link

stujo commented Dec 9, 2016

Ahh

Ahh thanks Dan, at the time instead of src/index.js I had added it to the end of config/polyfills.js :

if (!Array.includes) {
  require('core-js/fn/array/includes');
}

@sheerun
Copy link
Contributor

sheerun commented Jan 19, 2017

@gaearon Should babel-preset-env take care of polyfilling only needed built-ins?

@sheerun
Copy link
Contributor

sheerun commented Jan 19, 2017

ping @hzoo

@hzoo
Copy link

hzoo commented Jan 19, 2017

The way it works now it requires you to have import "babel-polyfill"; somewhere in your code.

only needed built-ins?

1 is not including polyfills based on the browsers, 2 is not including polyfills that aren't used in the codebase itself (would have to include node_modules/ 3rd party bundled code too if they expect the user to provide them). Seems to be a difficult thing to do so an open issue for it at babel/babel-preset-env#84 (most likely needs 2 passes?)

@sheerun
Copy link
Contributor

sheerun commented Jan 19, 2017

@hzoo I'd say 1 pass is enough if you use aickin's approach and modify his plugin to not output imports if target set of browsers already supports them.

@RangerMauve
Copy link

The issue brought up at the end of #914 where this doesn't work well for people who use react-native-web and want to import some JS-only React-Native dependencies.

This project already support this use case by aliasing react-native to react-native-web, why not go all the way and support dependencies?

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2017

@RangerMauve

The build times are already pretty slow. We won't be making them slower by compiling everything in node_modules. It is also not always safe (i.e. compiling a library with Babel can break it in some edge cases).

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2017

By the way polyfills are now documented here:

https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/template/README.md#supported-language-features-and-polyfills

So we can close this.

@gaearon gaearon closed this as completed Feb 16, 2017
thektan added a commit to thektan/trending that referenced this issue Jun 30, 2017
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants