Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

fix: do not match too many characters when removing sourceMappingURL … #286

Merged
merged 4 commits into from
Sep 22, 2020
Merged

fix: do not match too many characters when removing sourceMappingURL … #286

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d867e25
fix: do not match too many characters when removing sourceMappingURL …
schl3ck Sep 10, 2020
be26e4d
test(inline-option): don't break syntax when `devtool` is `eval-sourc…
schl3ck Sep 11, 2020
7a044cb
fix: sourceMappingURL regex matches whitespace and quotes after backs…
schl3ck Sep 11, 2020
e793510
test(inline-option): don't fail on correct multiline comment
schl3ck Sep 11, 2020
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
2 changes: 1 addition & 1 deletion src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ ${
}

// Matches only the last occurrence of sourceMappingURL
const innerRegex = /\s*[#@]\s*sourceMappingURL\s*=\s*([^\s'"]*)\s*/;
const innerRegex = /\s*[#@]\s*sourceMappingURL\s*=\s*((?:[^\s'"\\]|\\[^n])*(?:\\n)?)\s*/;
Copy link
Member

Choose a reason for hiding this comment

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

^n? Looks it is invalid

Copy link
Contributor Author

@schl3ck schl3ck Sep 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The idea was to match everything except all whitespace, quotes and \n literally (not the line feed character), but not a backslash on its own. It works like a negative lookahead but consuming characters. Another possible solution would be .*?(?=[\s'"]|\\n)(?:\\n)?, which actually looks a lot more readable (instead of (?:[^\s'"\\]|\\[^n])*(?:\\n)?)

And I've just noticed that with the current regex the match wouldn't stop when it encounters a whitespace or quote after a backslash, but it would have done before. Also the test will fail, if it is in a multiline comment. I will work on a fix. Thanks for not letting this slip past!


/* eslint-disable prefer-template */
const sourceMappingURLRegex = RegExp(
Expand Down
6 changes: 6 additions & 0 deletions test/__snapshots__/inline-option.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ exports[`"inline" option should work with "no-fallback" value and "esModule" wit

exports[`"inline" option should work with "no-fallback" value and "esModule" with "true" value: warnings 1`] = `Array []`;

exports[`"inline" option should work with "no-fallback" value and the "devtool" option ("eval-source-map" value): errors 1`] = `Array []`;

exports[`"inline" option should work with "no-fallback" value and the "devtool" option ("eval-source-map" value): result 1`] = `"{\\"postMessage\\":true,\\"onmessage\\":true}"`;

exports[`"inline" option should work with "no-fallback" value and the "devtool" option ("eval-source-map" value): warnings 1`] = `Array []`;

exports[`"inline" option should work with "no-fallback" value and the "devtool" option ("source-map" value): errors 1`] = `Array []`;

exports[`"inline" option should work with "no-fallback" value and the "devtool" option ("source-map" value): errors 2`] = `Array []`;
Expand Down
31 changes: 31 additions & 0 deletions test/inline-option.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,37 @@ describe('"inline" option', () => {
expect(getErrors(stats)).toMatchSnapshot('errors');
});

it('should work with "no-fallback" value and the "devtool" option ("eval-source-map" value)', async () => {
const compiler = getCompiler(
'./basic/entry.js',
{ inline: 'no-fallback' },
{ devtool: 'eval-source-map' }
);
const stats = await compile(compiler);
const result = await getResultFromBrowser(stats);
const moduleSource = getModuleSource('./basic/worker.js', stats);

expect(moduleSource.indexOf('inline.js') > 0).toBe(true);
expect(
moduleSource.indexOf('__webpack_public_path__ + "test.worker.js"') === -1
).toBe(true);
expect(
moduleSource.indexOf(
'sourceMappingURL=data:application/json;charset=utf-8;base64,'
) === -1
).toBe(true);
expect(
moduleSource.indexOf(
'//# sourceURL=webpack-internal:///./basic/worker.js'
) >= 0
).toBe(true);
expect(stats.compilation.assets['test.worker.js']).toBeUndefined();
expect(stats.compilation.assets['test.worker.js.map']).toBeUndefined();
expect(result).toMatchSnapshot('result');
expect(getWarnings(stats)).toMatchSnapshot('warnings');
expect(getErrors(stats)).toMatchSnapshot('errors');
});

it('should work with "no-fallback" value and the "devtool" option ("source-map" value)', async () => {
const compiler = getCompiler(
'./basic/entry.js',
Expand Down