fix(compiler-core): fix the variable prefix problem in the inline expression

[linzhe141]

close #11465
this pr playground

[github-actions]

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	90.1 kB	34.5 kB	31.1 kB
vue.global.prod.js	147 kB	54 kB	48 kB

Usages

Name	Size	Gzip	Brotli
createApp	49.6 kB	19.5 kB	17.8 kB
createSSRApp	53.2 kB	20.9 kB	19.1 kB
defineCustomElement	51.9 kB	20.2 kB	18.5 kB
overall	63.1 kB	24.5 kB	22.3 kB

[skirtles-code]

I found a couple of things while testing this, though I suspect they might be considered out of scope for this specific fix.

---

The variable for catch supports destructuring. e.g.:

try {
  throw new Error('blah')
} catch ({ message, stack }) {
  console.log(message)
  console.log(stack)
}

The MDN entry for catch has an example similar to this:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch#catch_binding

This doesn't seem to work with the fix proposed here, though judging by the error message it seems that it's failing much earlier in the parsing process.

---

The other thing I noticed is that the variable doesn't seem to be scoped correctly. It should just be scoped to the block, but it leaks out. It looks like this isn't just a problem with catch, it also happens for for loops. Examples:

try {
  // ...
} catch (err) {
  console.log(err)
}

// This `err` should come from the surrounding scope, it is not the same as the `err` inside `catch`.
console.log(err)

for (const msg of msgs) {
  console.log(msg)
}

// This shouldn't be the same `msg` that was inside the loop, as it used `const`.
// If the loop had used `var` it would be the same.
console.log(msg)

• Playground

[linzhe141]

I found a couple of things while testing this, though I suspect they might be considered out of scope for this specific fix.

The variable for catch supports destructuring. e.g.:

try {
  throw new Error('blah')
} catch ({ message, stack }) {
  console.log(message)
  console.log(stack)
}

The MDN entry for catch has an example similar to this:

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch#catch_binding

This doesn't seem to work with the fix proposed here, though judging by the error message it seems that it's failing much earlier in the parsing process.

The other thing I noticed is that the variable doesn't seem to be scoped correctly. It should just be scoped to the block, but it leaks out. It looks like this isn't just a problem with catch, it also happens for for loops. Examples:

try {
  // ...
} catch (err) {
  console.log(err)
}

// This `err` should come from the surrounding scope, it is not the same as the `err` inside `catch`.
console.log(err)

for (const msg of msgs) {
  console.log(msg)
}

// This shouldn't be the same `msg` that was inside the loop, as it used `const`.
// If the loop had used `var` it would be the same.
console.log(msg)

• Playground

Indeed, there are still a lot of problems here.

[skirtles-code]

I've just retested this. It seems that the catch variable is still leaking outside its block:

• Playground

It looks like the problem has been fixed for for loops, but I'm not sure whether that really belongs in the same PR. I only mentioned it originally because I thought it might be the same underlying cause as the leaking catch variable.

---

It seems that destructuring on the catch only works one level deep. e.g.:

• Playground

I wonder whether this should be handled recursively, maybe by calling extractIdentifiers?

[linzhe141]

I've just retested this. It seems that the catch variable is still leaking outside its block:

• Playground

It looks like the problem has been fixed for for loops, but I'm not sure whether that really belongs in the same PR. I only mentioned it originally because I thought it might be the same underlying cause as the leaking catch variable.

It seems that destructuring on the catch only works one level deep. e.g.:

• Playground

I wonder whether this should be handled recursively, maybe by calling extractIdentifiers?

thanks for your suggestions

[yyx990803]

Thanks for the PR, but the change seems unnecessarily complex - we just need to handle CatchClause here: 077a1ae#diff-f7f6060108ab33afc3fbe936e9452d30cc06a7509296150e8521a647895995bdR80

I partially reused the test you created in this PR.

for...of leaking should be handled in a separate commit.

[yyx990803]

For loop leak should not happen if the declaration is using let / const but should happen if using var. This is now properly handled in 67bb820

[linzhe141]

@yyx990803

If this is the case, should the msgs in for of be $setup.msg?

PR Maybe it should be like this

[yyx990803]

msg is declared in the scope so the output is expected.

In normal JavaScript the code is invalid and results in an error (accessing an in-scope const before initialization), Vue should not change that.