Reject sourcemap before sending it to uglify #63
Conversation
At some point today, our builds started failing with a sourcemap error
from this lib. Running the tests here makes it replicable:
```
● allow to disable source maps
DefaultsError: `sourcemap` is not a supported option
at DefaultsError.get (eval at <anonymous> (node_modules/uglify-js/tools/node.js:20:1), <anonymous>:71:23)
```
This fixes the error by rejecting the option uglify is now rejecting
before the whole thing crashes.
index.js
Outdated
| }) | ||
| reject(Object.assign({}, userOptions, { | ||
| sourceMap: userOptions.sourcemap !== false | ||
| }), [ "sourcemap" ]) |
There was a problem hiding this comment.
I would prefer to use delete operator.
There was a problem hiding this comment.
the Dependence of serialize-javascript update to 1.9.0, leads to uglify-js always fail,
the message is Error: "sourcemap is not a supported option"
There was a problem hiding this comment.
|
Could you add test please |
|
@TrySound how do you suggest I write a test for it? The only change here is that In terms of the params being sent to uglify being correct, the pre-existing tests should ensure that. I would even argue that "allow to disable source maps" covers this change, as it was failing before it. |
|
serialize-javascript upgrade in master gave me failing test. Please upgrade to get this change covered. |
|
Extracted the normalize in order to add a test for it, and upgraded serialize-javascript to 1.9.0. |
test/test.js
Outdated
|
|
||
| test("normalizeOptions removes 'sourcemap'", async () => { | ||
| const normalizedOptions = normalizeOptions({ sourcemap: true }) | ||
| expect(normalizedOptions).toEqual({ sourceMap: true }); |
There was a problem hiding this comment.
I don't need unit test. The whole plugin should work.
There was a problem hiding this comment.
I'll remove that commit then
|
Great! Thank you |
|
Published 6.0.3 |
|
Also I recommend to use rollup-plugin-terser instead. |
At some point today, our builds started failing with a sourcemap error
from this lib. Running the tests here makes it replicable:
This fixes the error by rejecting the option uglify is now rejecting before the whole thing crashes. I have no idea what's changed today, as the option rejection has been in uglify for ages: https://github.com/mishoo/UglifyJS2/blob/v3.6.0/lib/utils.js#L93