Implement @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining #13009
Conversation
JLHwung
added
PR: Bug Fix 🐛
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/44467/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 687ddb5:
|
JLHwung
changed the title
| "a?.[fn?.(...[], 0)]", // optional chain in property will be handled when traversed | ||
| "a?.(fn?.(...[], 0))", // optional chain in arguments will be handled when traversed | ||
| "class C { #c; p = obj?.#c(...[]) }", | ||
| ]; |
There was a problem hiding this comment.
Here are the positive / negative cases on detecting the affected code patterns.
| * @param {(NodePath<t.OptionalMemberExpression | t.OptionalCallExpression>)} path | ||
| * @returns {boolean} | ||
| */ | ||
| export function shouldTransform( |
There was a problem hiding this comment.
This routine determine whether we should transpile the affected optional chain.
...pread-parameters-in-optional-chaining/test/fixtures/assumption-noDocumentAll/basic/input.mjs
Show resolved
Hide resolved
|
|
||
| a?.b(...c, 1); | ||
|
|
||
| a?.b?.(...c, 1); |
There was a problem hiding this comment.
It would be enough to transform this to
(_a2$b = (_a2 = a)?.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);instead of
(_a2 = a) === null || _a2 === void 0 ? void 0 : (_a2$b = _a2.b) === null || _a2$b === void 0 ? void 0 : _a2$b.call(_a2, ...c, 1);We don't need to transform inner optional checks, only the one that directly affects the call expression.
There was a problem hiding this comment.
Yeah I mentioned that this PR is sub-optimal for nested chains. But because it's an edge case and we already have to maintain two optional chain transform, I prefer to leave it as-is.
Co-authored-by: Justin Ridgewell <justin+github@ridgewell.name>
github-actions
bot
added
the
outdated
This PR transpiles the following optional chaining usage when targeted to latest V8 derived browsers
see also code examples in the upstream V8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=11558.
The other unmentioned optional chaining usage will not be transpiled if your targets already have optional chaining support.
In this PR we export the optional chaining transform from
proposal-optional-chainingand apply the transform inbugfix-v8-spread-parameters-after-optional-chainingwhen we see a specific affected code patterns.Note that this may be sub-optimal for nested optional chains, i.e. in
fn ?. (...y, z) ?. x, both optional chaining?will be transpiled even though we could have only transpiled the first one. However I think since 1) it is an edge case and 2) we have another optional transform in class properties, the maintainability should win here.For reviewers: This PR depends on #13008, please review that one first.