-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
fix(source-maps): remove existing sourcemap links #665
Conversation
Some external libraries ship with sourcemaps with the dist package including a `sourcemapUrl` link. This link causes problems during bundling, and is specifically exposed by anonymous amd modules and SystemJS loader (RequireJS). This change modifies amodro to strip any existing `sourcemapURL` link from the source of a referenced library that is being bundled. closes aurelia#659, related to aurelia#624
72f8463
to
64bb7d7
Compare
Cool ! Any idea when this will be merged? 😄 |
We first need to run a couple of tests before we can merge it in. I can't say exactly when it will be merged though |
So I have tried with the @simonfox version but I got the same error in the console. Installation → npm i -g simonfox/cli#feature/%23659-systemjs-load-bug 4m 43s 81ms → 130
/usr/local/Cellar/nvm/0.31.1/versions/node/v8.1.2/bin/au -> /usr/local/Cellar/nvm/0.31.1/versions/node/v8.1.2/lib/node_modules/aurelia-cli/bin/aurelia-cli.js
/usr/local/Cellar/nvm/0.31.1/versions/node/v8.1.2/bin/aurelia -> /usr/local/Cellar/nvm/0.31.1/versions/node/v8.1.2/lib/node_modules/aurelia-cli/bin/aurelia-cli.js
+ [email protected] |
@RomainLanz I see that you've installed @simonfox's branch globally, but did you do it locally (in your project dir) as well? |
No, I forget to did that! Now it works great without any issue. I'll use this version to continue the project and get back here if I get any other issue. 👍 |
This does remove the sourcemap comment from the source, but the CLI won't put the sourcemap in a .map file next to the bundle either. |
@JeroenVinke submitted this to fix the immediate issue...are you able to point me to the code responsible for generating the |
Sure, this happens here |
@JeroenVinke I am closing this PR as it doesn't solve the problem in the right place. My colleague @dima-v has just opened a PR that fixes this issue properly and includes a bunch of improvements to bundle source maps. |
Thank you @simonfox |
Some external libraries ship with sourcemaps with the dist package including a
sourcemapUrl
link. This link causes problems during bundling, and is specifically exposed by anonymous amd modules and SystemJS loader (RequireJS). This change modifies amodro to strip any existingsourcemapURL
link from the source of a referenced library that is being bundled.closes #659, related to #624