8000 Prefer the last //# sourceMappingURL in a file instead of the first by papandreou · Pull Request #53 · stacktracejs/stacktrace-gps · GitHub
[go: up one dir, main page]

Skip to content

Prefer the last //# sourceMappingURL in a file instead of the first #53

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

Merged

Conversation

papandreou
Copy link
Contributor

Description

It seems like some Webpack setups (including mine) end up outputting several //# sourceMappingURL=... entries when both transpilation and bundling is in the picture.

In general it seems like the safest bet to go for the last one, as most tools tend to append the comment to the end of the file.
Thus the last one is most likely to pertain to the last operation that happened, be it transpilation, bundling, or uglification :)

Motivation and Context

In my Webpack setup, stacktrace-gps tries to fetch a non-existent source map because my bundle contains multiple //# sourceMappingURL=... entries. This worked fine before #33 was merged, but now stacktrace-gps prefers the first //# sourceMappingURL=... when the file contains more than one.

How Has This Been Tested?

In my own application that uses Webpack and via a new unit test.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • node_modules/.bin/jscs -c .jscsrc stacktrace-gps.js passes without errors
  • npm test passes without errors
  • I have read the contribution guidelines
  • I have updated the documentation accordingly
  • I have added tests to cover my changes

It seems like some Webpack setups (including mine) end up outputting
several //# sourceMappingURL=... entries when both transpilation and
bundling is in the picture.

In general it seems like the safest bet to go for the last one, as
most tools tend to append the comment to the end of the file.
Thus the last one is most likely to pertain to the last operation
that happened, be it transpilation, bundling, or uglification :)
@cartant
Copy link
cartant commented Aug 9, 2017

Can any guidance be offered as to if/when this PR will be merged? With tools like ng-cli the Webpack configuration is managed by other tools and if said tools effect bundles with multiple sourceMappingURL entries, there's not a lot that can be done to address the situation by the tools' users.

return _extractLocationInfoFromSourceMapSource(stackframe, sourceMapConsumer, sourceCache)
.then(resolve)['catch'](function() {
resolve(stackframe);
return this._getSourceMapConsumer(sourceMappingURL, defaultSourceRoot)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though this is unrelated to the change, I appreciate the cleanup here. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included that that in a separate commit so that I could tick the "node_modules/.bin/jscs -c .jscsrc stacktrace-gps.js passes without errors" box ;)

@@ -327,6 +327,21 @@ describe('StackTraceGPS', function() {
}
});

it('ignores all but the last sourceMappingURL in the file', function (done) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay tests!

@eriwen eriwen merged commit 331754f into stacktracejs:master Aug 12, 2017
cartant added a commit to cartant/rxjs-spy that referenced this pull request Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0