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

Revert Index.js to use ES5 code to avoid uglification errors #1

Merged
merged 1 commit into from May 1, 2018

Conversation

ChuckkNorris
Copy link
Contributor

@ChuckkNorris ChuckkNorris commented Apr 25, 2018

Currently, the index.js uses ES6 code which will cause a production build error for apps created using Create-React-App which does not support uglification of all ES6 code, particularly arrow functions.

Considering we don't actually gain anything from using arrow functions inside the index.js file, I propose we convert the arrow functions to ES5 functions.

image

I have tested the commit which resolves the uglification error.

@masaakim
Copy link
Owner

@ChuckkNorris There are a lot of libraries written by ES6 or later. If we create apps that depends on libraries written by ES6 with create-react-app, we cannot uglify the code, right?

@ChuckkNorris
Copy link
Contributor Author

It's not all ES6 code, it just doesn't handle the arrow functions correctly.

Either way, the proposed solution is to either unmount our create-react-app app, which prevents further version upgrades, remove your library as a dependency, continue manually modifying the file to ES5, or publish my own fork to NPM.

However, we really like your library, so if we could update it so we didn't have to modify it, that'd be ideal.

Please see issue:
facebook/create-react-app#1125

No, this is not supported because builds will get much slower. We don't recommend using libraries that don't precompile their code to ES5 but are supposed to be usable in ES5.

@masaakim
Copy link
Owner

masaakim commented May 1, 2018

OK 👍 FYI, It will support in create-react-app v2. facebook/create-react-app#3776

@masaakim masaakim merged commit 58fa193 into masaakim:master May 1, 2018
@masaakim
Copy link
Owner

masaakim commented May 1, 2018

I just published v1.0.1 :)

@ChuckkNorris
Copy link
Contributor Author

Awesome! Thanks, @morishitter!

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

Successfully merging this pull request may close these issues.

None yet

2 participants