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

Installing ember-fetch breaks source maps for prod builds #119

Closed
buschtoens opened this issue Jul 4, 2018 · 9 comments
Closed

Installing ember-fetch breaks source maps for prod builds #119

buschtoens opened this issue Jul 4, 2018 · 9 comments

Comments

@buschtoens
Copy link
Contributor

Demo: https://github.com/buschtoens/ember-fetch-sourcemap-bug

Installing ember-fetch breaks source maps for prod builds (i.e. builds with minification enabled).

During the build, the following warning is emitted:

[WARN] (broccoli-uglify-sourcemap) "ember-fetch.map" referenced in "assets/vendor.js" could not be found

Source map output without ember-fetch:

{"version":3,"sources":["vendor/ember-cli/vendor-prefix.js","vendor/loader/loader.js","vendor/ember-resolver/legacy-shims.js", ...

Source map output with ember-fetch:

{"version":3,"sources":["0"],"names":["window","Ember ...
@buschtoens
Copy link
Contributor Author

The problem was introduced with v4.0.0.

While warning about deprecations, the source map gets built correctly:

yarn add -D 'ember-fetch@<4' && ember build -prod && head -c 500 ./dist/assets/vendor*.map

@buschtoens
Copy link
Contributor Author

Which is weird, because according to v3.4.4...v4.0.0 there is no difference between v3.4.5 and v4.0.0. 🤔

@buschtoens
Copy link
Contributor Author

Oops, it's not v3.4.4...v4.0.0. I was tired yesterday, sorry 😄. Looking for the right version now.

@buschtoens
Copy link
Contributor Author

It's v4.0.0...v4.0.1.

Works:

yarn add -D ember-fetch@4.0.0 && ember build -prod && head -c 500 ./dist/assets/vendor*.map

Breaks:

yarn add -D ember-fetch@4.0.1 && ember build -prod && head -c 500 ./dist/assets/vendor*.map

Therefore #112 (#111) bricked it. 🙁

@buschtoens
Copy link
Contributor Author

buschtoens commented Jul 5, 2018

ember-fetch/index.js

Lines 128 to 130 in d25abf6

var polyfillTree = concat(new MergeTrees([find(expandedFetchPath), find(expandedAbortcontrollerPath)]), {
outputFile: 'ember-fetch.js'
});

broccoli-concat by default creates a source map and injects a sourceMappingURL comment into the file. When the file is eventually concatenated into vendor.js, something goes wrong and the sourceMappingURL is not processed and just stays as it is.

This means that later, when broccoli-uglify-sourcemap (L31) processes vendor.js, it finds the rogue sourceMappingURL comment and tries to load the ember-fetch.map, which of course at that point is not part of the tree anymore.

One possible fix is passing sourceMapConfig: { enabled: false } to broccoli-concat to disable source map generation and thus removing the sourceMappingURL comment.

@buschtoens
Copy link
Contributor Author

I guess the file ends up here and broccoli-concat itself is probably not aware of source maps when joining files.

So I guess passing sourceMapConfig: { enabled: false } is indeed the correct solution to this problem.

buschtoens referenced this issue in buschtoens/ember-fetch Jul 5, 2018
Fixes #119.

When the ember-fetch.js file is eventully pulled into vendor.js and minifcation with source map generation is enabled, a rogue sourceMappingURL comment sneaks in and breaks everything. This PR removes that rogue comment.
@xg-wang
Copy link
Member

xg-wang commented Jul 9, 2018

@stefanpenner @tchak see comment here broccolijs/broccoli-concat#123 (comment)
Any thoughts about this?

@xg-wang
Copy link
Member

xg-wang commented Jul 10, 2018

Under repo https://github.com/buschtoens/ember-fetch-sourcemap-bug

rm yarn.lock
yarn install
ember b -prod

No more Building[WARN] (broccoli-uglify-sourcemap) "ember-fetch.map" referenced in "assets/vendor.js" could not be found

@mhluska
Copy link
Contributor

mhluska commented Nov 22, 2018

So the solution here was to nuke the yarn.lock file? 🤣

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

No branches or pull requests

3 participants