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

@babel/traverse noScope can't path.remove() #11128

Closed
liuxingbaoyu opened this issue Feb 11, 2020 · 6 comments · Fixed by #11136
Closed

@babel/traverse noScope can't path.remove() #11128

liuxingbaoyu opened this issue Feb 11, 2020 · 6 comments · Fixed by #11136
Labels

Comments

@liuxingbaoyu
Copy link
Contributor

@liuxingbaoyu liuxingbaoyu commented Feb 11, 2020

When I upgraded from v7.7.7 to v7.8.4, I found that there was a problem with the noScope option of @babel/traverse.

Error message:

e:\mypath\babel\node_modules\@babel\traverse\lib\path\removal.js:39
  Object.keys(bindings).forEach(name => this.scope.removeBinding(name));
                                                   ^

TypeError: Cannot read property 'removeBinding' of null
    at e:\mypath\babel\node_modules\@babel\traverse\lib\path\removal.js:39:52
    at Array.forEach (<anonymous>)
    at NodePath._removeFromScope (e:\mypath\babel\node_modules\@babel\traverse\lib\path\removal.js:39:25)
    at NodePath.remove (e:\mypath\babel\node_modules\@babel\traverse\lib\path\removal.js:22:8)
    at AssignmentExpression (e:\mypath\babel\test.js:6:14)
    at NodePath._call (e:\mypath\babel\node_modules\@babel\traverse\lib\path\context.js:55:20)
    at NodePath.call (e:\mypath\babel\node_modules\@babel\traverse\lib\path\context.js:42:17)
    at NodePath.visit (e:\mypath\babel\node_modules\@babel\traverse\lib\path\context.js:90:31)
    at TraversalContext.visitQueue (e:\mypath\babel\node_modules\@babel\traverse\lib\context.js:112:16)
    at TraversalContext.visitSingle (e:\mypath\babel\node_modules\@babel\traverse\lib\context.js:84:19)

Code to reproduce the problem:

var traverse = require("@babel/traverse").default;
var parser = require("@babel/parser");

traverse(parser.parse("a=1"), {
    AssignmentExpression(path) {
        path.remove();
    },
    noScope: true,
});
@babel-bot

This comment has been minimized.

Copy link
Collaborator

@babel-bot babel-bot commented Feb 11, 2020

Hey @liuxingbaoyu! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@liuxingbaoyu

This comment has been minimized.

Copy link
Contributor Author

@liuxingbaoyu liuxingbaoyu commented Feb 11, 2020

I'm sorry, I'm such a fool. It should be that I have modified it before, this is not a regression error, it is just a common bug. Sorry!

My solution:
Replace this with

  if (!this.opts.noScope) {
    this._removeFromScope();
  }
@liuxingbaoyu liuxingbaoyu changed the title @babel/traverse noScope has a regression bug @babel/traverse noScope can't path.remove() Feb 11, 2020
@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Feb 11, 2020

Ok, it's not a problem!

@JLHwung

This comment has been minimized.

Copy link
Contributor

@JLHwung JLHwung commented Feb 12, 2020

Closing it as the OP's issue is resolved.

@JLHwung JLHwung closed this Feb 12, 2020
@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Feb 12, 2020

@JLHwung The issue was resolved by modifying our code 😛

@liuxingbaoyu Do you want to open a PR?

@liuxingbaoyu

This comment has been minimized.

Copy link
Contributor Author

@liuxingbaoyu liuxingbaoyu commented Feb 13, 2020

@nicolo-ribaudo OK, let me try!

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

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.