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

Set SourceMap Value based on Webpack Config #95

Conversation

nicholascm
Copy link

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • [] test update .... working on this
  • typo fix
  • metadata update

Motivation / Use-Case

Basically when switching over to use terser-webpack-plugin I wasted some time trying to figure out why sourcemaps weren't working when I had configured it in the plugins for webpack, only to realize I needed to pass sourceMap: true to our TerserPlugin.

I saw that the webpack defaults actually already do this, and it seems to make sense to do it here as well and remove the config.

#71

Breaking Changes

With this change, the default for sourceMap defers to the webpack configuration devtool or plugins that use the SourceMapDevToolPlugin to define sourcemaps instead of defaulting to false.

Additional Info

set the sourceMap property by checking webpack config - easier to configure terser
@jsf-clabot
Copy link

jsf-clabot commented May 23, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

Merging #95 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   98.23%   98.25%   +0.01%     
==========================================
  Files           5        5              
  Lines         283      286       +3     
  Branches      113      116       +3     
==========================================
+ Hits          278      281       +3     
  Misses          5        5
Impacted Files Coverage Δ
src/index.js 98.1% <100%> (+0.03%) ⬆️

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 6d7aaa7...ba54a13. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Thanks for PR, it is breaking change, so we will merge this for v2.0, anyway thanks for PR

@alexander-akait
Copy link
Member

Partial fixed for 2.0.0, it is invalid use

compiler.options.plugins.some(
              (p) => p instanceof SourceMapDevToolPlugin
            ))

because plugin doesn't support some devtool values, we try to fix it in webpack@5 and then fix it here

Anyway thanks for PR

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.

3 participants