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

generate higher quality SourceMaps #109

Merged
merged 3 commits into from Jul 31, 2019
Merged

Conversation

sokra
Copy link
Member

@sokra sokra commented Jul 31, 2019

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes webpack/webpack#8302

Mappings generated when combining bundle source map with terser source map are not very fine, which leads to validation errors and low quality source maps.

There are many causes:

  • webpack generates mappings that map a line to a line
  • The SourceMap spec has no idea of: "this is a copied line"

We had a similar problem with webpack replacements (using the ReplaceSource from webpack-sources) and a good fix turned out to be:

Assume that mappings, where generates source and original source are identical for a mapping, map source code char by char.

With this assumption mappings don't have to be super exact as long code stays equal. This allows to apply the high quality terser SourceMap to the lower quality webpack SourceMap without loosing quality.

webpack-sources now implements with kind of assumption (I call it "identity" mappings) for SourceMapSource. This PR changes terser-webpack-plugin to use it.

Test snapshots are updated with new mappings that include more column information.

Breaking Changes

This should have no breaking changes. SourceMaps are very similar, but with additional column information.

Additional Info

For reviewing see only second commit. First commit changes snapshot format to be easier reviewable.

@codecov
Copy link

codecov bot commented Jul 31, 2019

Codecov Report

Merging #109 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #109   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files           5        5           
  Lines         283      283           
  Branches      113      113           
=======================================
  Hits          279      279           
  Misses          4        4
Impacted Files Coverage Δ
src/index.js 98.06% <ø> (ø) ⬆️
src/minify.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0d8317...e7a6f1d. Read the comment docs.

@@ -55,7 +55,7 @@ Object {
"code": "function foo(f){if(f)return bar()}",
"error": undefined,
"extractedComments": Array [],
"map": "{\\"version\\":3,\\"sources\\":[\\"test1.js\\"],\\"names\\":[\\"foo\\",\\"x\\",\\"not_called1\\"],\\"mappings\\":\\"AAAA,SAASA,IAAIC,GACT,GACIA,EAAA,OACAC\\"}",
"map": "{\\"version\\":3,\\"sources\\":[\\"test6.js\\"],\\"names\\":[\\"foo\\",\\"x\\",\\"bar\\"],\\"mappings\\":\\"AAAA,SAASA,IAAIC,GAAK,GAAIA,EAAK,OAAOC\\"}",
Copy link
Member Author

Choose a reason for hiding this comment

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

We no longer apply the inputSourceMap in the worker, so this snapshot has changed.

It was incorrect anyway since not_called1 should not be in the SourceMap as it is dropped by terser.

Error: \\"version\\" is a required argument.",
]
`;
exports[`when options.sourceMap matches snapshot for a single \`true\` value (\`devtool\` is \`source-map\`) and source map invalid: errors 1`] = `Array []`;
Copy link
Member Author

Choose a reason for hiding this comment

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

This no longer crashes, but a warning is still displayed.

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.

Webpack is producing invalid sourcemaps?
2 participants