Skip to content
This repository was archived by the owner on Feb 1, 2025. It is now read-only.

Remove old bindings when hoisting vars#368

Merged
benjamn merged 3 commits intofacebook:masterfrom
nicolo-ribaudo:babel-6923
Mar 20, 2019
Merged

Remove old bindings when hoisting vars#368
benjamn merged 3 commits intofacebook:masterfrom
nicolo-ribaudo:babel-6923

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Contributor

@nicolo-ribaudo nicolo-ribaudo commented Feb 28, 2019 •
edited
Loading

Fixes babel/babel#6923, fixes babel/babel#8899, fixes babel/babel#8525

When hoisting variables, regenerator transformslet a = 2; to a = 2; and then injects var a at the beginning of the generator function scope.
Babel isn't really good at keeping the scope synchronized with the AST (and we won't fix it for a long time; it's probably the hardest thing to fix): for this reason, sometimes it throws when injecting var a because it thinks that let a is still there.
Manually removing the old bindings fixes the bug.

Note that this bug is only present when using regenerator-transform without @babel/plugin-transform-block-scoping (and thus it isn't reproducible when using regenerator-preset), because @babel/plugin-transform-block-scoping transforms let->var and vars can be duplicated.

From babel/babel#6923:

async function foo() {
	(async function (number) {
		const tmp = number
	})
}

Before this PR:

/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/scope/index.js:344
      throw this.hub.buildError(id, `Duplicate declaration "${name}"`, TypeError);
      ^

TypeError: undefined: Duplicate declaration "tmp" (This is an error on an internal node. Probably an internal error.)
    at File.buildCodeFrameError (/home/nicolo/Documenti/dev/babel/babel/packages/babel-core/lib/transformation/file/file.js:261:12)
    at Scope.checkBlockScopedCollisions (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/scope/index.js:344:22)
    at Scope.registerBinding (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/scope/index.js:501:16)
    at Scope.registerDeclaration (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/scope/index.js:441:14)
    at Object.Declaration (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/scope/index.js:125:12)
    at NodePath._call (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/path/context.js:53:20)
    at NodePath.call (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/path/context.js:40:17)
    at NodePath.visit (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/path/context.js:88:12)
    at TraversalContext.visitQueue (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/context.js:118:16)
    at TraversalContext.visitMultiple (/home/nicolo/Documenti/dev/babel/babel/packages/babel-traverse/lib/context.js:85:17)

After this PR:

function foo() {
  return regeneratorRuntime.async(function foo$(_context2) {
    while (1) switch (_context2.prev = _context2.next) {
      case 0:
        (function f(number) {
          var tmp;
          return regeneratorRuntime.async(function f$(_context) {
            while (1) switch (_context.prev = _context.next) {
              case 0:
                tmp = number;

              case 1:
              case "end":
                return _context.stop();
            }
          });
        });

      case 1:
      case "end":
        return _context2.stop();
    }
  });
}

@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Feb 28, 2019
Closed
benjamn
benjamn approved these changes Mar 7, 2019
View reviewed changes
@benjamn @nicolo-ribaudo
Update test/tests.transform.js
27f9973 
Co-Authored-By: nicolo-ribaudo <nicolo.ribaudo@gmail.com>
@benjamn benjamn merged commit f44b30d into facebook:master Mar 20, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the babel-6923 branch March 20, 2019 14:55
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Jul 15, 2019
Merged
@fossa-robszumski fossa-robszumski bot mentioned this pull request Dec 16, 2025
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@benjamn benjamn benjamn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed

Projects

None yet

Milestone

No milestone

3 participants

@nicolo-ribaudo @benjamn @facebook-github-bot