Implement minify-builtins plugin #410
Conversation
|
Thoughts: I think we could also help constant-propagation by evaluating the expressions if the arguments are constants ? let a = 1.8, b = 2.5; // (2) becomes deadcode
let x = Math.floor(Math.max(a, b)); // (1) evaluate
foo(x);could be transformed to foo(2); |
|
@boopathi I like the idea in general, I believe it shouldn't cause any issues.. Shall i create a new issue for that and add it as part of new PR? |
|
Part of this PR would be great. |
|
Handled the optimisations. |
| @@ -0,0 +1,79 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
The way babel does this is to maintain only [String, Number, Math]
https://github.com/babel/babel/blob/f8ffe03/packages/babel-traverse/src/path/evaluation.js#L5
So, I think it's relatively safe to assume that Math.x will be a side-effect free evaluation except Math.random
There was a problem hiding this comment.
Ahh good one.. What about Date.x?
There was a problem hiding this comment.
We can chuck the Date for now.
There was a problem hiding this comment.
Cool.. then will update
|
|
||
| const expName = getExpressionName(callee); | ||
| if (isBuiltin(expName)) { | ||
| const result = evaluate(path); |
There was a problem hiding this comment.
we have to deopt when we have side-effecty evaluate-able arguments
for example,
Math.floor((foo(), 1));
| @@ -0,0 +1,51 @@ | |||
| # babel-plugin-transform-built-ins | |||
There was a problem hiding this comment.
should we just call it babel-plugin-minify-builtins
There was a problem hiding this comment.
Hmmm.. dono really..
There was a problem hiding this comment.
Let's just change it to babel-plugin-minify-builtins
| const evaluate = require("babel-helper-evaluate-path"); | ||
|
|
||
|
|
||
|
|
| } | ||
|
|
||
| const expName = getExpressionName(callee); | ||
| if (isBuiltin(expName)) { |
There was a problem hiding this comment.
should check if it's a computed property
Will fail for Math[max]() - will assume it as Math.max().
let max = "floor";
Math[max](1.5);
|
|
||
| if (t.isIdentifier(object)) result += object.name; | ||
| if (t.isMemberExpression(object)) result += memberToString(object); | ||
| if (t.isIdentifier(property)) result += property.name; |
There was a problem hiding this comment.
should check for computed properties. same as above.
vigneshshanmugam
changed the title
|
@boopathi Addressed all the issues. |
vigneshshanmugam
force-pushed
the
built-ins
branch
from
173bfcf
to
56154cd
Compare
|
Is this already available in |
|
Nope.. Not released yet. |
fixes #363