Don't rename a function expression's name to match a parameter shadowing it - #4330
Don't rename a function expression's name to match a parameter shadowing it#4330jzhan-canva wants to merge 2 commits into
Conversation
…ing it.
Safari 16 and earlier treat a function expression's name as one of its
parameters, so they reject a function whose non-simple parameter list binds that
same name. Given
var foo = function bar({bar = baz}) { ... };
RenameVars renames the function name and the parameter to a single name, and the
resulting `function a({bar: a = b})` fails to parse with "SyntaxError: Duplicate
parameter 'a' not allowed in function with destructuring parameters".
Give the function name its own assignment when a parameter shadows it and the
parameter list is non-simple, so the two get different names. Simple parameter
lists are unaffected, since they don't trigger the bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I have signed CLA but claude hasn't? |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
For clarity, does this problem only occur if the input JS already exhibits the shadowing issue or does JS Compiler sometimes introduce the shadowing issue when it wasn't present before? Also, in what version of Safari was this issue fixed? |
|
CC won't introduce it — only when the input already has the shadowing. Worth stressing that the construct is perfectly legal JavaScript; the function name and the parameter are separate bindings and every other engine handles it. It's only WebKit that rejects it. We want to rely on closure compiler to keep it out of our production output. On the Safari version: the JSC fix landed in trunk as 256478@main on 2022-11-08. It can be reproduced in safari <16.3 |
Older versions of Safari treat a function expression's name as one of its parameters, so they reject a function whose non-simple parameter list binds that same name:
See https://bugs.webkit.org/show_bug.cgi?id=220517 and https://bugs.webkit.org/show_bug.cgi?id=247433.
The function name and the parameter are separate bindings, so renaming them apart is free — the parameter shadows the function name throughout the function, making the name unreferenceable.
This gives the function name its own
Assignmentwhen a parameter shadows it and the parameter list is non-simple.assignNamesreserves every name it generates, so a distinct key is enough to guarantee a distinct name. Simple parameter lists are left alone, sincefunction a(a) {}doesn't trigger the bug and sharing the name is cheaper.recordPseudoNameneeded the same distinction, otherwise--debugoutput kept the collision.Tested:
bazel test //...— 431/431 targets pass.