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.

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

Issue applying babel-plugin-transform-remove-undefined in babel7-beta.40 #803

Closed
diervo opened this issue Feb 25, 2018 · 4 comments
Closed

Comments

@diervo
Copy link

diervo commented Feb 25, 2018

Input Code

(function () {
  function test() {
    let a;
  }
}());

Actual Output

TypeError: Cannot read property 'isPure' of null
    at Object.checkPath (/Users/dval/Sites/minify/node_modules/@babel/traverse/lib/path/lib/virtual-types.js:120:23)
    at NodePath.(anonymous function) [as isPure] (/Users/dval/Sites/minify/node_modules/@babel/traverse/lib/path/index.js:205:24)
    at isPureAndUndefined (/Users/dval/Sites/minify/packages/babel-plugin-transform-remove-undefined/lib/index.js:28:13)
    at PluginPass.VariableDeclaration (/Users/dval/Sites/minify/packages/babel-plugin-transform-remove-undefined/lib/index.js:180:21)

Expected Output

(function(){})();

Details

I trace it down to the following code in babel-plugin-transform-remove-undefined :

When visiting VariableDeclaration node, and accessing the path for its property init, that path should have a scope pointing to the parent function declaration. Instead scope is null.

Not sure if is a bug in babel.

diervo added a commit to diervo/minify that referenced this issue Feb 25, 2018
Fix: babel#803
Altough not sure if the root cause is on babel.
@diervo
Copy link
Author

diervo commented Feb 25, 2018

Some more digging: it seems that the issue on remove-undefined is a side-effect from when also enabling babel-plugin-minify-constant-folding.

A trivial workaround is to add a guard for !rval.scope || !rval.isPure() in as the first par of the expression insideisPureAndUndefined() function, but you guys will probably know better?

FYI, for more accurate testing I updated all dependencies to babel40
#804

@diervo
Copy link
Author

diervo commented Feb 25, 2018

Also I believe the constant-folding is why the CI is failing. Its all related with scope property being undefined.

@boopathi
Copy link
Member

I kinda drilled it down to simplify. So, maybe both the transforms are.

In some places of babel-minify, we avoid the babel's re-queueing (path.replaceWith) deliberately and mutate the node directly. Maybe these transforms in simplify and constant folding does this.

Also, interestingly, the mangle's re-queueing had to be turned off for babel-minify to work with babel-7 - c0c712d#diff-18ba673c600d34ebd7daef11f67829f9R438

@boopathi
Copy link
Member

This is merged to master. Thanks for the fix @diervo .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants