Skip to content
This repository was archived by the owner on Dec 5, 2019. It is now read-only.

feat: support devtool (options.sourceMap) #81

Closed
wants to merge 4 commits into from
Closed

feat: support devtool (options.sourceMap) #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
003c0c6
added logic to support auto-defaulting sourceMap based on compiler.op…
hulkish Jul 12, 2017
4839dc8
added auto sourcewMap support for devtool === "hidden-source-map"
hulkish Jul 12, 2017
1e95e02
added snapshot tests for auto sourceMap feature
hulkish Jul 12, 2017
7f7eeed
applied requested change to loop syntax, fixed title of snapshot test
hulkish Jul 13, 2017
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
4 changes: 4 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ class UglifyJsPlugin {
uglifyOptions.output = uglifyOptions.output || {};
}

if (typeof this.options.sourceMap === 'undefined' && ['sourcemap', 'source-map', 'hidden-source-map'].indexOf(compiler.options.devtool) > -1) {
Copy link
Member

@michael-ciniawsky michael-ciniawsky Jul 12, 2017

Choose a reason for hiding this comment

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

Is the typeof this.options.sourceMap === 'undefined' necessary here ?

Copy link
Contributor Author

@hulkish hulkish Jul 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, if we're talking current usage. Because it can be user defined as true or false. This basically only auto-sets the sourceMap option value if u aren't manually overriding it.

This way, we aren't breaking current usage of sourceMap option

Copy link
Member

@michael-ciniawsky michael-ciniawsky Jul 13, 2017

Choose a reason for hiding this comment

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

If devtool should be automatically supported, the previous this.options.sourceMap 'doesn't matter' as it is overridden to true anyways in case compilation.options.devtool is set ? Maybe I'm missing a case here 🙃

Copy link
Member

@michael-ciniawsky michael-ciniawsky Jul 13, 2017

Choose a reason for hiding this comment

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

And if I missed one if (typeof this.options.sourceMap === "undefined") => if (!this.options.sourceMap) ?

Copy link
Contributor Author

@hulkish hulkish Jul 13, 2017

Choose a reason for hiding this comment

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

well, honestly I'm not certain of the history of this option.. I was just being careful to not break it's current usage.

That said, i see your point and im alright with removing the exposing of this option all together. Up to u.

Copy link
Contributor Author

@hulkish hulkish Jul 14, 2017

Choose a reason for hiding this comment

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

@michael-ciniawsky what u think?

this.options.sourceMap = true;
}

compiler.plugin('compilation', (compilation) => {
if (this.options.sourceMap) {
compilation.plugin('build-module', (moduleArg) => {
Expand Down
Loading