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

improve support of malformed/missing/broken sourcemaps #23

Open
stefanpenner opened this issue Feb 5, 2016 · 14 comments
Open

improve support of malformed/missing/broken sourcemaps #23

stefanpenner opened this issue Feb 5, 2016 · 14 comments
Assignees
Labels

Comments

@stefanpenner
Copy link
Collaborator

transplanted from : ember-cli/ember-cli-terser#4 (comment)


@stefanpenner

Here's an updated repo which reproduces the issue: https://github.com/johnnyshields/ember-sourcemap-issue

I believe I found the problem: ember-cli-uglify requires 3rd-party bower libs to have their magic sourcemap at the BEGINNING of their source, so that Ember CLI can concatenate them like this:

//# sourceMappingURL=lib1.map
lib1.js code

lib2.js code   // this lib doesn't have a sourcemap

//# sourceMappingURL=lib3.map
lib3.js code

If a lib declares the magic comment at the END of its file, the following happens:

lib1.js code

//# sourceMappingURL=lib1.map      ember gets con
lib2.js code

Because Ember CLI evaluates the maps on the CONCATENATED file, here it apparently gets confused and thinks the source map is for lib2 when it's actually for lib1.

So, infact in the original post of this thread, select2 is not the issue--whichever JS comes BEFORE select2 is the issue, but error thrown looks like it's select2.

My sample repo uses sprintf.js as an offender which has magic comment at the end. If you manually edit the sprintf.js to have the magic comment at the beginning, everything works.

@stefanpenner
Copy link
Collaborator Author

Strange, I am pretty sure at the end works. I believe this test even illustrates that https://github.com/ef4/broccoli-uglify-sourcemap/blob/master/test/fixtures/inside/with-upstream-sourcemap.js#L22

@johnnyshields
Copy link

Try my example, you can see clearly that if you manually edit bower_components/sprintf/dist/sprintf.min.js to move the magic comment to before the code it works.

@johnnyshields
Copy link

(Which indicates that the sourcemap itself isn't malformed / broken / missing)

@johnnyshields
Copy link

(Though there should be a story for handling the case of missing / broken / malformed as well)

@stefanpenner
Copy link
Collaborator Author

Interesting, sounds like we have 2 separate things:

  • fix the comment ordering issue
  • support broken/malformed case more elegantly

@johnnyshields if you have time to (at the least) provide a failing test on this repo, it will be something I suspect I can look at this weekend and try to sort out. That would just give me a good run-way. (unless you are interested in taking it to completion, or iterating)

@johnnyshields
Copy link

EDIT: Ignore this comment and see next one

@stefanpenner 100% I'm not sure but it looks like your existing tests each file is individually fed into UglifyWriter.prototype.processFile one-by-one; if I log the inFile arg I see this:

C:\workspace\broccoli-uglify-sourcemap\test\fixtures\inside\with-upstream-source
map.js
C:\workspace\broccoli-uglify-sourcemap\test\fixtures\no-upstream-sourcemap.js

C:\workspace\broccoli-uglify-sourcemap\test\fixtures\inside\with-upstream-source
map.js
C:\workspace\broccoli-uglify-sourcemap\test\fixtures\no-upstream-sourcemap.js
C:\workspace\broccoli-uglify-sourcemap\test\fixtures\no-upstream-sourcemap.js

C:\workspace\broccoli-uglify-sourcemap\test\fixtures\inside\with-upstream-source
map.js
C:\workspace\broccoli-uglify-sourcemap\test\fixtures\no-upstream-sourcemap.js

If I do the same in my demo repo, I see this:

C:\workspace\ember-sourcemap-issue\tmp\concat_with_maps-output_path-Pk
pGeNKQ.tmp\assets\vendor.map
C:\workspace\ember-sourcemap-issue\tmp\concat_with_maps-output_path-ZURfxS8e.tmp
\assets\ember-sourcemap-issue.map
C:\workspace\ember-sourcemap-issue\tmp\concat_with_maps-output_path-EpWgw17P.tmp
\assets\vendor.css.map
C:\workspace\ember-sourcemap-issue\tmp\broccoli_merge_trees-output_path-nEoOELSI
.tmp\assets\ember-sourcemap-issue.js
C:\workspace\ember-sourcemap-issue\tmp\broccoli_merge_trees-output_path-nEoOELSI
.tmp\assets\vendor.js

So it appears in the real-world app situation, UglifyWriter.prototype.processFile is receiving vendor.js which has already been concatenated, and that's where the pre/post confusion occurs.

I'm not familiar enough with all the details yet to convert this into a test case, but perhaps the above explains why your existing tests aren't failing?

@johnnyshields
Copy link

@stefanpenner OK I believe I've gotten close to the issue. I've analyzed the intermediate vendor.map file which is fed into broccoli-uglify-sourcemap's UglifyWriter.prototype.processFile() method during the compilation chain. Using my demo repo, when the sprintf.js file has the magic comment at the beginning, I get a vendor.map which contains this:

\\ vendor.map -- GOOD when sprintf.js has comment at beginning

{"version":3,"sources":[...,"bower_components/ember-data/ember-data.prod.js","bower_components/sprintf/dist/sprintf.min.js","bower_components/bootstrap/dist/js/bootstrap.js",...

However, when the magic comment comes at end of the file, I see this:

\\ vendor.map -- BAD when sprintf.js has comment at end

{"version":3,"sources":[...,"bower_components/ember-data/ember-data.prod.js","../src/sprintf.js","bower_components/bootstrap/dist/js/bootstrap.js",...

So the difference is in GOOD version sprintf.js has a full path, while BAD sprintf.js has a relative path.

You can reproduce the same thing by logging the inFile arg of UglifyWriter.prototype.processFile() then going to your /tmp/ directory and looking at it's map.

I believe this is enough information to fix the issue; I think the problem is NOT in broccoli-uglify-sourcemap but in whatever lib generates the concatenated vendor.map sourcemap.

Unfortunately I won't have time this week to investigate this further or write a test case, but I believe that this information should be enough for you to nail down the exact issue.

@stefanpenner
Copy link
Collaborator Author

@johnnyshields it also seems like min.js is lost in the second version.

@johnnyshields
Copy link

You're right, when magic comment at the top it's the same as if it were not there all, i.e. Ember CLI uses the sprint.min.js file itself. (Magic comment at top is actually not allowed according to sourcemap spec. TLDR; I was wrong about magic comment, it's not the problem.

Moreover I'm not sure there's any issue with the relative paths, because if I delete the sprintf.js (the unminifed one) I get a different error which indicates it can't find the file (which it could previously.)

Error: ENOENT, no such file or directory 'C:\workspace\ember-sourcemap-issue\tmp
\concat_with_maps-input_base_path-8oK0kWyb.tmp\0\bower_components\sprintf\src\sp
rintf.js'

I validated https://raw.githubusercontent.com/alexei/sprintf.js/master/dist/sprintf.min.js using https://sourcemaps.io/ and it didn't have any errors. I have no clue what's going on here.

@johnnyshields
Copy link

Added PR w/ test case for this issue here: #24

@johnnyshields
Copy link

@stefanpenner I have another lead. It seems like the vendor.js / map generated during Ember CLI compilation and fed into broccoli-uglify-sourcemap is invalid according to sourcemaps.io, and moreover the validation error corresponds to the error we see in Ember.

Note that the individual sourcemaps which are compiled into the build are not invalid themselves, so its reasonable to think something is incorrect during the concatenation causing the output to be malformed.

@stefanpenner
Copy link
Collaborator Author

@johnnyshields awesome thanks for the continued deep dive. As my work also wants this to work ideally, I can likely take some time this week during the day to try and sort this out.

@stefanpenner stefanpenner self-assigned this Feb 7, 2016
@johnnyshields
Copy link

Update: I've made a test here: broccolijs/broccoli-concat#50 for what I believe to be the root cause--broccoli-concat is generating an invalid vendor.js/vendor.map which broccoli-uglify-sourcemap chokes on later in the build process.

@Cryrivers
Copy link

I found another case is that some third-party libraries have Base64 sourcemap embedded in the source file which also crash the build process.

To reproduce, you can run bower install -S json-bigint, then try building with flag -prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants