Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support marking a call as pure #1448

Closed
wants to merge 4 commits into from
Closed

Support marking a call as pure #1448

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
58036cd
Support marking a call as pure
kzc Jan 29, 2017
d35b92c
remove [#@]__PURE__ hint from comment when pure call dropped
kzc Feb 21, 2017
883ce81
have [@#]__PURE__ regex work with node 0.10.x
kzc Feb 21, 2017
b943c0d
have `#__PURE__` hint only work when compress `side_effects` option e…
kzc Feb 21, 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
18 changes: 18 additions & 0 deletions lib/compress.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,25 @@ merge(Compressor.prototype, {
def(AST_Constant, return_false);
def(AST_This, return_false);

function has_pure_annotation(node) {
if (node.pure !== undefined) return node.pure;
var pure = false;
var comments, last_comment;
if (node.start
&& (comments = node.start.comments_before)
&& comments.length
&& /[@#]__PURE__/.test((last_comment = comments[comments.length - 1]).value)) {
last_comment.value = last_comment.value.replace(/[@#]__PURE__/g, ' ');
Copy link
Contributor

Choose a reason for hiding this comment

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

@kzc any reason why the #__PURE__ comment has to be the last one? why this isnt checking all node.start.comments_before?

If im gonna write i.e. a babel plugin which annotates calls for me it might interfere with other annotations causing i.e. such situation

var A = /*#__PURE__*/ /* @class */ function() { /* ... */ }();

and thus preventing it from being dropped as unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any reason why the #__PURE__ comment has to be the last one? why this isnt checking all node.start.comments_before?

No particular reason other than efficiency. It probably should check all comments. PR is welcome.

Be aware that it's unlikely to be back-ported to uglify-js@2.x which is widely used but whose code base is frozen. So you'd probably want to list #__PURE__ in the last comment to aid legacy uglify users.

Copy link
Contributor

Choose a reason for hiding this comment

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

No particular reason other than efficiency. It probably should check all comments. PR is welcome.

PR is comming then. :)

Be aware that it's unlikely to be back-ported to uglify-js@2.x which is widely used but whose code base is frozen. So you'd probably want to list #PURE in the last comment to aid legacy uglify users.

While at the moment of adding the comment I can ensure its added as the last comment, I cannot be sure that no other transformation change it. I wouldnt be overly concern about old version of the uglify. Life goes on and library's consumers should upgrade sooner or later :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should mention that the following would still work with current uglify versions:

var A = /* @__PURE__ @class */ function() { /* ... */ }();

That's all the proposed babel plugin would have to do to retain backwards compatibility with uglify.

pure = true;
}
return node.pure = pure;
}

def(AST_Call, function(compressor){
if (compressor.option("side_effects") && has_pure_annotation(this)) {
compressor.warn("Dropping __PURE__ call [{file}:{line},{col}]", this.start);
return false;
}
var pure = compressor.option("pure_funcs");
if (!pure) return true;
if (typeof pure == "function") return pure(this);
Expand Down
60 changes: 60 additions & 0 deletions test/compress/issue-1261.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
pure_function_calls: {
options = {
evaluate : true,
conditionals : true,
comparisons : true,
side_effects : true,
booleans : true,
unused : true,
if_return : true,
join_vars : true,
cascade : true,
negate_iife : true,
}
input: {
// pure top-level IIFE will be dropped
// @__PURE__ - comment
(function() {
console.log("iife0");
})();

// pure top-level IIFE assigned to unreferenced var will not be dropped
var iife1 = /*@__PURE__*/(function() {
console.log("iife1");
function iife1() {}
return iife1;
})();

(function(){
// pure IIFE in function scope assigned to unreferenced var will be dropped
var iife2 = /*#__PURE__*/(function() {
console.log("iife2");
function iife2() {}
return iife2;
})();
})();

// comment #__PURE__ comment
bar(), baz(), quux();
a.b(), /* @__PURE__ */ c.d.e(), f.g();
}
expect: {
var iife1 = function() {
console.log("iife1");
function iife1() {}
return iife1;
}();

baz(), quux();
a.b(), f.g();
}
expect_warnings: [
"WARN: Dropping __PURE__ call [test/compress/issue-1261.js:17,8]",
"WARN: Dropping side-effect-free statement [test/compress/issue-1261.js:17,8]",
"WARN: Dropping __PURE__ call [test/compress/issue-1261.js:30,37]",
"WARN: Dropping unused variable iife2 [test/compress/issue-1261.js:30,16]",
"WARN: Dropping side-effect-free statement [test/compress/issue-1261.js:28,8]",
"WARN: Dropping __PURE__ call [test/compress/issue-1261.js:38,8]",
"WARN: Dropping __PURE__ call [test/compress/issue-1261.js:39,31]"
]
}
15 changes: 15 additions & 0 deletions test/mocha/minify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,19 @@ describe("minify", function() {
assert.strictEqual(code, "var a=function(n){return n};");
});
});

describe("#__PURE__", function() {
it("should drop #__PURE__ hint after use", function() {
var result = Uglify.minify('//@__PURE__ comment1 #__PURE__ comment2\n foo(), bar();', {
fromString: true,
output: {
comments: "all",
beautify: false,
}
});
var code = result.code;
assert.strictEqual(code, "// comment1 comment2\nbar();");
});
});

});