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

Add core-js@2 dependency that is required by transpiled outputs of Babel #757

Closed

Conversation

sebinsua
Copy link
Contributor

@sebinsua sebinsua commented Oct 7, 2019

Many of the packages use Babel configurations which create various core-js@2 imports in their transpiled outputs.

This dependency should be installed by these packages in order that they work at runtime. Also, while it is possible to recommend to consumers to manually install core-js@2 themselves, this is fragile because if they depend on any libraries which use core-js@3 it is likely that this could end up hoisted to the top of the node_modules directory, which would cause the wrong core-js to be picked up and the code to fail to execute.

@finos-admin
Copy link
Member

Thank you for your pull request and welcome to our community! We require contributors to sign a Contributor License Agreement and we don't seem to have CLAs on file for these contributors to the Pull Request: (@sebinsua). In order for your PR to be reviewed and merged, please follow the directions at the link above.

Project team: please do not merge this Pull Request until Foundation staff have confirmed that a CLA is in place for the new contributor(s) listed above.

If you still have any questions or just don't know what to do next, please email [email protected]. /CC @finos-admin

@sebinsua
Copy link
Contributor Author

sebinsua commented Oct 8, 2019

I've noticed that everybody's JS builds are failing because Babel doesn't understand the import.meta.url syntax.

There seem to be two fixes to this. You can either fix the problem by adding support to Babel for this syntax, or stopping emscripten from producing it in its output (see: emscripten-core/emscripten#9234). What do people prefer?

@texodus
Copy link
Member

texodus commented Oct 8, 2019

Thanks for the PR!

We're undergoing an upgrade to our build infrastructure at the moment - the changes for the new containers are in #755. When we merge this branch, I will rebase your changes and they should pass.

@texodus
Copy link
Member

texodus commented Oct 10, 2019

@sebinsua I've rebased this branch on top of master in #757, so closing this PR.

@texodus texodus closed this Oct 10, 2019
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.

3 participants