Skip to content

Commit

Permalink
Change default export to named one
Browse files Browse the repository at this point in the history
Now you should import this plugin like this

```js
import { uglify } from 'rollup-plugin-uglify';

export default {
  ...
  plugins: [uglify()]
  ...
}
```

Added babel code frame so now syntax errors should look prettier
```
    > 1 | var = 1
        |    ^ Name expected

Error: Error transforming bundle with 'uglify' plugin: Name expected
```

Started testing on node 6, 8 and the latest one.
  • Loading branch information
TrySound committed May 19, 2018
1 parent 6e46a12 commit cd8f9f1
Show file tree
Hide file tree
Showing 9 changed files with 822 additions and 176 deletions.
5 changes: 5 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"presets": [
["@babel/env", { "targets": { "node": 6 } }]
]
}
5 changes: 1 addition & 4 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1 @@
node_modules/
coverage
npm-debug.log
dist
node_modules
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
language: node_js
node_js:
- "node"
- "8"
- "6"
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ npm i rollup-plugin-uglify -D

```js
import { rollup } from 'rollup';
import uglify from 'rollup-plugin-uglify';
import { uglify } from 'rollup-plugin-uglify';

rollup({
entry: 'main.js',
Expand Down
14 changes: 8 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
const minify = require("uglify-js").minify;
const { codeFrameColumns } = require("@babel/code-frame");
const { minify } = require("uglify-js");

function uglify(userOptions, minifier) {
if (minifier === undefined) {
minifier = minify;
}
function uglify(userOptions, minifier = minify) {
const options = Object.assign({ sourceMap: true }, userOptions);

return {
Expand All @@ -12,11 +10,15 @@ function uglify(userOptions, minifier) {
transformBundle(code) {
const result = minifier(code, options);
if (result.error) {
const { message, line, col: column } = result.error;
console.error(
codeFrameColumns(code, { start: { line, column } }, { message })
);
throw result.error;
}
return result;
}
};
}

module.exports = uglify;
exports.uglify = uglify;
11 changes: 8 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@
"author": "Bogdan Chadkin <trysound@yandex.ru>",
"license": "MIT",
"dependencies": {
"@babel/code-frame": "^7.0.0-beta.47",
"uglify-js": "^3.3.25"
},
"devDependencies": {
"jest": "^22.1.4",
"prettier": "^1.10.2",
"rollup": "^0.54.1"
"@babel/core": "^7.0.0-beta.47",
"@babel/preset-env": "^7.0.0-beta.47",
"babel-core": "^7.0.0-bridge.0",
"babel-jest": "^22.4.4",
"jest": "^22.4.4",
"prettier": "^1.12.1",
"rollup": "^0.59.1"
}
}
4 changes: 2 additions & 2 deletions test/fixtures/unminified.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";
var a = 5;
window.a = 5;

if (a < 3) {
if (window.a < 3) {
console.log(4);
}
22 changes: 10 additions & 12 deletions test/test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
const assert = require("assert");
const rollup = require("rollup").rollup;
const readFile = require("fs").readFileSync;
const uglify = require("../");
const { rollup } = require("rollup");
const { readFileSync: readFile } = require("fs");
const { uglify } = require("../");

test("minify", async () => {
const bundle = await rollup({
input: "test/fixtures/unminified.js",
plugins: [uglify()]
});
const result = await bundle.generate({ format: "cjs" });
expect(Object.keys(result)).toHaveLength(2);
expect(result.code).toEqual('"use strict";var a=5;a<3&&console.log(4);\n');
expect(result.code).toEqual('"use strict";window.a=5,window.a<3&&console.log(4);\n');
expect(result.map).toBeFalsy();
});

Expand All @@ -23,7 +22,6 @@ test("minify via uglify options", async () => {
banner: "/* package name */",
format: "cjs"
});
expect(Object.keys(result)).toHaveLength(2);
expect(result.code).toEqual('/* package name */\n"use strict";\n');
expect(result.map).toBeFalsy();
});
Expand All @@ -34,7 +32,6 @@ test("minify with sourcemaps", async () => {
plugins: [uglify()]
});
const result = await bundle.generate({ format: "cjs", sourcemap: true });
expect(Object.keys(result)).toHaveLength(2);
expect(result.map).toBeTruthy();
});

Expand Down Expand Up @@ -66,14 +63,15 @@ test("throw error on uglify fail", async () => {
const bundle = await rollup({
input: "test/fixtures/failed.js",
plugins: [
uglify({}, () => ({
error: Error("some error")
}))
{
transformBundle: () => ({ code: "var = 1" })
},
uglify()
]
});
await bundle.generate({ format: "es" });
expect(true).toBeFalsy();
} catch (err) {
expect(err.toString()).toMatch(/some error/);
} catch (error) {
expect(error.toString()).toMatch(/Name expected/);
}
});
Loading

5 comments on commit cd8f9f1

@papandreou
Copy link

Choose a reason for hiding this comment

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

Any particular reason why the breaking API change had to happen? It makes this plugin look a bit weird alongside other rollup plugins: unexpectedjs/unexpected@4ea106f

@TrySound
Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't use default exports anymore. They are bad with IDEs and hides the actual name. I wouldn't say it's a problem for you.

@jhildenbiddle
Copy link

Choose a reason for hiding this comment

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

Any chance we can get a CHANGELOG.md with details added to the repo?

No disagreement on the rational for the change, but nobody wants to dig through commit logs and comment threads to track down details on a breaking change.

@TrySound
Copy link
Owner Author

Choose a reason for hiding this comment

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

@jhildenbiddle
Copy link

Choose a reason for hiding this comment

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

Ahh. There it is. πŸ˜„

Thanks, @TrySound .

Please sign in to comment.