-
-
Notifications
You must be signed in to change notification settings - Fork 177
test: add test for MultiCompiler usage
#74
Conversation
fc62311 to
f9b569b
Compare
test/helpers.js
Outdated
| const eventLength = Array.isArray(_plugins[name]) ? _plugins[name].length : 0; | ||
|
|
||
| if (eventLength > currentLength) { | ||
| Object.assign(aggregate, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just aggregate[name] = eventLength?
| } | ||
|
|
||
| exports.removeCWD = function removeCWD(str) { | ||
| export function countPlugins({ _plugins }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the usecase here ? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kovensky @michael-ciniawsky Was for the test I hadn't added at first to this pr. But, I have since added it: https://github.com/webpack-contrib/uglifyjs-webpack-plugin/pull/74/files#diff-0570d5214a7cbff246a7ad90d119f15c
src/index.js
Outdated
|
|
||
| apply(compiler) { | ||
| const requestShortener = new RequestShortener(compiler.context); | ||
| const requestShortener = new RequestShortener(compiler.context || process.cwd()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is process.cwd() save to use in most cases ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, i have a regression test to add that uses this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michael-ciniawsky - Agree here. We should get context without using process. || window || w/e
| } | ||
|
|
||
| exports.removeCWD = function removeCWD(str) { | ||
| export function countPlugins({ _plugins }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the usecase here ? :)
MultiCompiler support
|
path.resolve()? |
For |
f9b569b to
351cbfa
Compare
|
@michael-ciniawsky yes, |
…ce it seems to have magically fixed itself
264114e to
50bf746
Compare
MultiCompiler supportMultiCompiler usage
Fixes #73
Update: So, it looks like this issue just magically fixed itself. That, said - I'd like to still include the added tests and upgrade to
[email protected].