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

Prevent npm preload module to break dev bundles #1003

Merged
merged 1 commit into from
May 29, 2018
Merged

Conversation

m-mujica
Copy link
Contributor

@m-mujica m-mujica commented May 26, 2018

Closes #947

Part of the problem is that the module to preload the npm packages is added at the top of the bundle. So we run into situations like:

{ foo: "/*" };define("foo", function() { var bar = "*/" });

then the amd extension can't see the "define" because the comments regex matches a bunch of the module definition:

screen shot 2018-05-26 at 11 05 36 am

The solution I came up with was to push the module with the package.jsons to the bottom of the bundle, that way the first thing the regex matches is the actual AMD modules.

Placing the preloader at the bottom shouldn't matter since it gets executed right away.

@stealjs stealjs deleted a comment from coveralls May 26, 2018
@stealjs stealjs deleted a comment from coveralls May 26, 2018
@stealjs stealjs deleted a comment from coveralls May 26, 2018
@m-mujica m-mujica force-pushed the dev-bundle-minify branch 2 times, most recently from a751d91 to 0154773 Compare May 26, 2018 14:54
@m-mujica m-mujica force-pushed the dev-bundle-minify branch from 0154773 to 614272f Compare May 26, 2018 17:00
@m-mujica m-mujica changed the title Add breaking test for minified dev bundles issue Prevent npm preload module to break dev bundles May 26, 2018
@matthewp
Copy link
Member

Sounds good to me, nice job.

@m-mujica m-mujica merged commit 7a20fe5 into master May 29, 2018
@m-mujica m-mujica deleted the dev-bundle-minify branch May 29, 2018 16:24
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.

2 participants