Helper evaluate path #645
Helper evaluate path #645
Conversation
|
Can you update the README to explain what this does now? |
|
path.evaluate in babel doesn't handle some of the cases like this The helper takes care of this scenario and also for references used before/after bindings and handles if its within same/diff scope. We need to enable this on simplify and DCE instead of babel evaluate. |
vigneshshanmugam
force-pushed
the
helper-evaluate-path-0
branch
from
20667a8 to
03d1e63
Compare
| // https://github.com/babel/babel/blob/master/packages/babel-traverse/src/path/evaluation.js | ||
| // modified for Babili use | ||
| function evaluateIdentifier(path) { | ||
| if (!path.isReferencedIdentifier()) { |
There was a problem hiding this comment.
This will help in catching bugs where the refId path is replaced with something else. This has been an issue in DCE+mangle previously.
| scopePath.skip(); | ||
| }, | ||
| ReferencedIdentifier(idPath) { | ||
| const binding = idPath.scope.getBinding(idPath.node.name); |
There was a problem hiding this comment.
we can remove this as well. evaluate identifier takes care of this already.
There was a problem hiding this comment.
This is required as well. For babel's evaluate to take care of globals. Here we ignore globals. In evaluatePath we depot it. If a referencedId makes it to evaluateId, we have to deopt instead of simply ignoring it. Though that code looks like it's never reached, it will be useful for detecting side-effects from other transformations.
| ? fnParent.get("body") | ||
| : fnParent.get("body").get("body"); | ||
|
|
||
| const state = { |
There was a problem hiding this comment.
lets extract this usage before init in to separate function.
| `, | ||
| ` | ||
| function foo() { | ||
| bar(void 0); |
There was a problem hiding this comment.
should be referenceError right not void 0
There was a problem hiding this comment.
yes. I'll skip this test for now. This doesn't go through evaluate.
There was a problem hiding this comment.
Ahh that babel bug? can you put the link to the bug as well?
There was a problem hiding this comment.
The transformation is not a babel-bug. It's a bug in one-use-replacements in DCE. The parsing is a babel-bug.
|
we need to use this evaluate in simplify as well in patterns matching case, this bug still exists #574 |
|
Simplify requires a lot of changes in updating scope. Postponing it for later. |
|
We are doing the same checks/ optimisations already here https://github.com/babel/babili/blob/master/packages/babel-plugin-minify-dead-code-elimination/src/index.js#L314 Some of the cases we discussed like function () {
var a = b;
console.log(b);
b = 10;
}are handled to an extent. |
|
Yeah. That code doesn't go through evaluate. It's a one time used variable replacement that doesn't have any side-effects. evaluate takes care of finding values. |
No description provided.