Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

Clarify note about loading polyfills only once #282

Merged
merged 2 commits into from
Apr 13, 2017

Conversation

darahak
Copy link
Contributor

@darahak darahak commented Apr 13, 2017

Reading the note, I wasn't sure if it could cause errors or not, and I had to look around for a little while.
Now it will say explicitly that loading babel-polyfill more than once will throw an error.

@hzoo
Copy link
Member

hzoo commented Apr 13, 2017

I did put

Only use require("babel-polyfill"); once in your whole app

But if this makes it clearer, we can do that. I can remove multiple imports if it's in the same file but if you actually import polyfill multiple times then yes it will error. Actually now that I think about it, using useBuiltIns won't import babel-polyfill so that error message won't be hit

@existentialism
Copy link
Member

Yeah, I'd prefer if we just combine the statements, something like:

NOTE: Only use require("babel-polyfill"); once in your whole app. Multiple imports or requires of babel-polyfill will throw an error. We recommend creating a single entry file that only contains the require statement.

@codecov-io
Copy link

codecov-io commented Apr 13, 2017

Codecov Report

Merging #282 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #282   +/-   ##
=======================================
  Coverage   93.03%   93.03%           
=======================================
  Files           4        4           
  Lines         201      201           
  Branches       59       59           
=======================================
  Hits          187      187           
  Misses          9        9           
  Partials        5        5

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cad1b2b...f676161. Read the comment docs.

@darahak
Copy link
Contributor Author

darahak commented Apr 13, 2017

@existentialism Your proposal has better phrasing. I would like to keep the rationale though.

@existentialism
Copy link
Member

existentialism commented Apr 13, 2017

Sure:

NOTE: Only use require("babel-polyfill"); once in your whole app. Multiple imports or requires of babel-polyfill will throw an error since it can cause global collisions and other issues that are hard to trace. We recommend creating a single entry file that only contains the require statement.

(Feel free to modify, just wanted to provide a quick example)

@darahak
Copy link
Contributor Author

darahak commented Apr 13, 2017

Thanks.
I updated this with your suggestion.

@existentialism existentialism merged commit 8bca925 into babel:master Apr 13, 2017
@existentialism
Copy link
Member

@darahak thanks!

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.

4 participants