bublejs / buble

https://buble.surge.sh
MIT License
871 stars 64 forks source link

A bug when destructuring in a for .. of loop #258

Open mourner opened 4 years ago

mourner commented 4 years ago

Minimal test case:

{
  let a;
}
for (const [a] of b) a();

Produces:

{
    var a;
}
for (var i = 0, list = b; i < list.length; i += 1) {
    var ref = list[i];
    var a = ref[0];

    a$1();
}

Note that the a$1 reference at the end is incorrect, since this symbol is not defined.

thomaswelter commented 4 years ago

You can sidestep this issue for now by using var instead of let or const, or by using assignment like for([a] of b) a()

The problem seems to be that in BlockStatement.js a new alias is created but the Node is kept the same. Then later when destructure is called in ForOfStatement.js the resolveName is passed that node in destructure.js at destructureIdentifier causing the old identifier to be used.

I can create a fix this but I need a little guidance. I don't quite understand where this needs to be changed. I see two parts that I think might be important:

  1. destructureIdentifier in destructure.js: resolveName should take into account the either an updated Node or receive the declaration with the rewritten = true property set.
  2. In BlockStatement.js at transpileBlockScopedIdentifiers: declaration.name = alias could be changed to declaration.node.name. But this is not a full solution because it creates problems in other places (such as property access).