Skip to content

Find the *last* sourceMappingURL in the source document. #48

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
merged 2 commits into from
May 18, 2015

Conversation

mprobst
Copy link
Collaborator

@mprobst mprobst commented Apr 9, 2015

This fixes odd behaviour for source files that contain the literal string
'//# sourceMappingURL=' somewhere in the actual source code.

@mprobst
Copy link
Collaborator Author

mprobst commented May 1, 2015

@evanw ping?

@julien-f
Copy link
Collaborator

I am no source map expert but I believe source maps can be specified anywhere in code and can perfectly be defined at the top of the file.

Is #58 enough for your issue?

@mprobst mprobst force-pushed the match-last-sourceMappingURL branch from 9766df8 to 3f56d90 Compare May 17, 2015 15:09
@mprobst
Copy link
Collaborator Author

mprobst commented May 17, 2015

@julien-f this actually supports having multiple source mapping URLs in one file, it just changes the behaviour to always use the last one. As transpilers usually append a source mapping URL at the end of the file, the last mapping URL is much more likely to be the right one to use. So I think this change is still useful.

The change to not find quotes might be correct (though I'm not sure quotes need to be escaped in URIs/URLs) and help, but if somebody has code handling e.g. ... sourceMappingURL=http://base/" + myHref, it won't help, so this should still go in.

@julien-f
Copy link
Collaborator

It makes sense.

I think it should be mentioned in the README and maybe we should update the examples to have the directive at the end?

@mprobst
Copy link
Collaborator Author

mprobst commented May 18, 2015

@julien-f done, please take another look.

@julien-f
Copy link
Collaborator

LGTM but I don't understand the test: I see only one source map directive, am I missing something?

mprobst added 2 commits May 18, 2015 17:33
This fixes odd behaviour for source files that contain the literal string
'//# sourceMappingURL=' somewhere in the actual source code.
@mprobst
Copy link
Collaborator Author

mprobst commented May 18, 2015

Added a clarifying comment - the test driver function actually adds the secondary sourceMappingURL comment.

@mprobst
Copy link
Collaborator Author

mprobst commented May 18, 2015

Thanks for the review, merging this now.

@mprobst mprobst force-pushed the match-last-sourceMappingURL branch from f4f1638 to e6e77ab Compare May 18, 2015 15:35
@mprobst mprobst merged commit e6e77ab into evanw:master May 18, 2015
@mprobst mprobst deleted the match-last-sourceMappingURL branch May 18, 2015 15:36
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