Skip to content

support charset in data url #76

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 1 commit into from
Jun 17, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions source-map-support.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ var fileContentsCache = {};
// Maps a file path to a source map for that file
var sourceMapCache = {};

// Regex for detecting source maps
var reSourceMap = /^data:application\/json[^,]+base64,/;

function isInBrowser() {
return ((typeof window !== 'undefined') && (typeof XMLHttpRequest === 'function'));
}
Expand Down Expand Up @@ -97,10 +100,10 @@ function retrieveSourceMap(source) {

// Read the contents of the source map
var sourceMapData;
var dataUrlPrefix = "data:application/json;base64,";
if (sourceMappingURL.slice(0, dataUrlPrefix.length).toLowerCase() == dataUrlPrefix) {
if (reSourceMap.test(sourceMappingURL)) {
// Support source map URL as a data url
sourceMapData = new Buffer(sourceMappingURL.slice(dataUrlPrefix.length), "base64").toString();
var rawData = sourceMappingURL.slice(sourceMappingURL.indexOf(',') + 1);
sourceMapData = new Buffer(rawData, "base64").toString();
sourceMappingURL = null;
} else {
// Support source map URLs relative to the source URL
Expand Down
24 changes: 24 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,3 +391,27 @@ it('missing source maps should also be cached', function(done) {
'1', // The retrieval should only be attempted once
]);
});

/* The following test duplicates some of the code in
* `compareStackTrace` but appends a charset to the
* source mapping url.
*/
it('finds source maps with charset specified', function() {
var sourceMap = createMultiLineSourceMap()
var source = [ 'throw new Error("test");' ];
var expected = [
'Error: test',
/^ at Object\.exports\.test \(.*\/line1\.js:1001:101\)$/
];

fs.writeFileSync('.generated.js', 'exports.test = function() {' +
source.join('\n') + '};//@ sourceMappingURL=data:application/json;charset=utf8;base64,' +
new Buffer(sourceMap.toString()).toString('base64'));
try {
delete require.cache[require.resolve('./.generated')];
require('./.generated').test();
} catch (e) {
compareLines(e.stack.split('\n'), expected);
}
fs.unlinkSync('.generated.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you create/delete .generated.js yourself instead of using compareStackTrace()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

compareStackTrance() is hard coded to use data:application/json;base64, (aka. no charset). I didn't want to change it because we should test for both. I didn't want to add an extra parameter because I felt that it added too much clutter vs. the benefit, thus I just inlined the relevant parts in this test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough.

Would you mind adding a small comment above the test explaining that this is some duplicated code and why it is reasonable to do so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

});