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

Do not bundle babel-polyfill with library. Let it be an external peer dependency #109

Open
anandanand84 opened this issue Feb 13, 2018 · 2 comments · May be fixed by #125
Open

Do not bundle babel-polyfill with library. Let it be an external peer dependency #109

anandanand84 opened this issue Feb 13, 2018 · 2 comments · May be fixed by #125

Comments

@anandanand84
Copy link

Shipping babel polyfill with the library doesnt play nice with other libraries.

https://github.com/transcranial/keras-js/blob/master/src/index.js#L1

If an application depends on multiple libraries and everything ships with babel-polyfill it will cause

Uncaught Error: only one instance of @babel/polyfill is allowed

Even in cases where the other application doesnt ship with babel polyfill but depend on babel-polyfill, it will fail since the the babel polyfill will be loaded later when this(keras-js) dependency is loaded.

Only solution is to load the babel polyfill first from a cdn or the end application add it as a dependency in their build process

@levithomason
Copy link

levithomason commented Feb 14, 2018

Agreed. Shipping the polyfill causes unwarranted side-effects for apps loading keras-js as well. It would be ideal if keras-js provided pre-compiled targets. This way users could opt-in to loading the package that suited their needs.

Another option is to rollback the future features relying on the polyfill. I'm not sure what the scope of that would be in this repo.

@tiansivive
Copy link

tiansivive commented Mar 2, 2018

Please fix this, it's causing me major headaches, since keras-je is being included as a third party dependency by a lib i'm using

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 a pull request may close this issue.

3 participants