-
-
Notifications
You must be signed in to change notification settings - Fork 179
fix(index): relax default asset name {RegExp}
(options.test
)
#251
Conversation
src/index.js
Outdated
@@ -20,7 +20,7 @@ class UglifyJsPlugin { | |||
|
|||
const { | |||
uglifyOptions = {}, | |||
test = /\.js$/i, | |||
test = /.((js)?)(\?[a-z0-9]+)?$/i, |
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.
The .
needs to be escaped (?)
- /.js
+ /\.js...
Why ((js)?)
('nested' capturing groups) ?
d9f4e24
to
424d7af
Compare
@michael-ciniawsky done 👍 |
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.
👍
{RegExp}
(options.test
)
{RegExp}
(options.test
){RegExp}
(options.test
)
Released in |
@evilebottnawi was it intentional to match the .j extension? Also an empty query should probably be allowed. Maybe |
The common default at this stage of the build is a JS asset ending in
What's the 'use case' for an |
I'm not following you, maybe I didn't make myself clear. The Regex in the commit will match files with a .js and a .j extension which I don't think is desirable or the intention. There's not a use case per say, just in case someone uses a variable substitution that evaluates to an empty string. |
@evilebottnawi @michael-ciniawsky any comment on the Regex? edit. This demonstrates the incorrect behaviour - https://regex101.com/r/SJm7SO/1 |
@jazdw oh, yes, you are right we should fix it 👍 |
Issues