-
Notifications
You must be signed in to change notification settings - Fork 71
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
Adding Kinto to create-react-app breaks yarn build #849
Comments
By the way, this works but is not a solution: ▶ yarn add kinto@11.2.2
▶ yarn build
...
Compiled successfully.
... |
Actually, this works too: ▶ yarn add kinto@12.0.0
▶ yarn build
...
Compiled successfully.
... So the regression is in 12.0.0...12.0.1. |
I don't really know what I'm looking for but the diff between the
I.e. in kinto 12.0.1 there are these for additional files:
|
More of a solution is to run |
At this point I don't know if the resolution is to make a loud "BREAKING CHANGES" warning in the changelog or if there's something we need to do within this repo to avoid this. |
Yeah, like I said in our standup this morning, this is probably my fault because I'm the one who updated babel to the new version, and 12.0.1 is the first version that includes those changes. It wasn't really clear to me (both with the old |
Oh that old issue. I would dare to have opinions if it wasn't for the fact that so much has changed with babel in recent months. Also, there are two separate discussions going on. Whether the kinto.min.js shipped to unkpg should have all the polyfills baked in or whether users are expected to polyfill that themselves. Then, there's the whole ES5 vs ES6 discussion. If you treeshake or webpack or parcel bundle kinto.js into your project you probably don't want any of the "kinto.js dist" code at all or any of the polyfills. Having said that, where does that leave projects like create-react-app that can only really bundle ES5 packaged modules. |
Per discussion on IRC, we believe that our approach is indeed the correct one and that this is fixed now. |
Steps to reproduce:
Then edit
src/App.js
to something like this:(not sure if this is necessary because I don't know if CRA's treeshaking would remove the dependency when building)
The text was updated successfully, but these errors were encountered: