New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reuse computed key memoiser #16159
Reuse computed key memoiser #16159
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55969/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we eventually port these changes to maybeGenerateMemoised
itself?
The challenge of porting these changes to Currently For point 1, For point 2, the API should return whether the expression is of the format I am wondering if we need a broader helper to cover these steps, e.g. |
const isUidReference = | ||
t.isIdentifier(computedKey.node) && scope.hasUid(computedKey.node.name); | ||
const isMemoiseAssignment = | ||
computedKey.isAssignmentExpression({ operator: "=" }) && | ||
t.isIdentifier(computedKey.node.left) && | ||
scope.hasUid(computedKey.node.left.name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't understand why hasUid
is used, can you explain it? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. The hasUid
checks whether the identifier is generated from scope.generateUidIdentifier
, if it is, it is very likely generated by official Babel plugins, intended to be a memoiser. If not, it is from user code and we should handle it more conservatively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future maybe we can use Set
or WeakSet
to achieve this.
I was going to suggest to re-use this new logic in other places, but looking out out tests I only found https://github.com/nicolo-ribaudo/babel/blob/3caeeb18833c4f04ccd3218af492e6fd3a072818/packages/babel-plugin-transform-class-properties/test/fixtures/nested-class/super-call-in-decorator/output.js where this would be useful ( |
@nicolo-ribaudo Yes there are more optimizable paths. As you mentioned it helps the decorator memoization, we will see more such patterns once we start interleaving computed keys and decorator evaluations. For example, let brandCheckC;
function captureClassBrandCheck(fn) {
brandCheckC = fn;
return v => v
}
class C {
[Symbol.iterator]() {}
@captureClassBrandCheck(o => #p in o) #p
} Currently Babel transforms it to var _computedKey, _dec, _init_p;
let brandCheckC;
function captureClassBrandCheck(fn) {
brandCheckC = fn;
return v => v;
}
_computedKey = _toPropertyKey(Symbol.iterator);
_dec = captureClassBrandCheck(o => #p in o);
class C {
static {
[_init_p] = _applyDecs(this, [[_dec, 0, "p", o => o.#p, (o, v) => o.#p = v]], [], 0, _ => #p in _).e;
}
[_computedKey]() {}
#p = _init_p(this);
} which is invalid because the private brand check var _computedKey, _dec, _init_p;
let brandCheckC;
function captureClassBrandCheck(fn) {
brandCheckC = fn;
return v => v;
}
class C {
static {
[_init_p] = _applyDecs(this, [[_dec, 0, "p", o => o.#p, (o, v) => o.#p = v]], [], 0, _ => #p in _).e;
}
[_computedKey = _toPropertyKey(Symbol.iterator), _dec = captureClassBrandCheck(o => #p in o), _computedKey]() {}
#p = _init_p(this);
} Now if we further transpile the result above to ES5, we should inform the class transform that Admittedly this case is not very practical, as users can just capture the brand check from the |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@babel/traverse](https://babel.dev/docs/en/next/babel-traverse) ([source](https://github.com/babel/babel)) | resolutions | patch | [`7.23.5` -> `7.23.7`](https://renovatebot.com/diffs/npm/@babel%2ftraverse/7.23.5/7.23.7) | --- ### Release Notes <details> <summary>babel/babel (@​babel/traverse)</summary> ### [`v7.23.7`](https://github.com/babel/babel/blob/HEAD/CHANGELOG.md#v7237-2023-12-29) [Compare Source](babel/babel@v7.23.6...v7.23.7) ##### 🐛 Bug Fix - `babel-traverse` - [#​16191](babel/babel#16191) fix: Crash when removing without `Program` ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helpers`, `babel-plugin-proposal-decorators` - [#​16180](babel/babel#16180) fix: Class decorator `ctx.kind` is wrong ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-plugin-proposal-decorators` - [#​16170](babel/babel#16170) Fix decorator initProto usage in derived classes ([@​JLHwung](https://github.com/JLHwung)) - `babel-core` - [#​16167](babel/babel#16167) Avoid unpreventable `unhandledRejection` events ([@​nicolo-ribaudo](https://github.com/nicolo-ribaudo)) ##### 🏠 Internal - `babel-helper-create-class-features-plugin` - [#​16186](babel/babel#16186) chore: Update deps ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators` - [#​16177](babel/babel#16177) Merge decorators into class features ([@​JLHwung](https://github.com/JLHwung)) ### [`v7.23.6`](https://github.com/babel/babel/blob/HEAD/CHANGELOG.md#v7236-2023-12-11) [Compare Source](babel/babel@v7.23.5...v7.23.6) ##### 👓 Spec Compliance - `babel-generator`, `babel-parser`, `babel-types` - [#​16154](babel/babel#16154) Remove `TSPropertySignature.initializer` ([@​fisker](https://github.com/fisker)) - `babel-helpers`, `babel-plugin-proposal-decorators`, `babel-plugin-transform-class-properties`, `babel-plugin-transform-class-static-block`, `babel-plugin-transform-runtime`, `babel-preset-env`, `babel-runtime-corejs2`, `babel-runtime-corejs3`, `babel-runtime`, `babel-types` - [#​16139](babel/babel#16139) Apply `toPropertyKey` on decorator context name ([@​JLHwung](https://github.com/JLHwung)) ##### 🐛 Bug Fix - `babel-generator` - [#​16166](babel/babel#16166) fix: Correctly indenting when `retainLines` is enabled ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helpers`, `babel-plugin-proposal-explicit-resource-management` - [#​16150](babel/babel#16150) `using`: Allow looking up `Symbol.dispose` on a function ([@​odinho](https://github.com/odinho)) - `babel-plugin-proposal-decorators`, `babel-plugin-transform-class-properties` - [#​16161](babel/babel#16161) Ensure the `[[@​@​toPrimitive]]` call of a decorated class member key is invoked once ([@​JLHwung](https://github.com/JLHwung)) - [#​16148](babel/babel#16148) Support named evaluation for decorated anonymous class exp ([@​JLHwung](https://github.com/JLHwung)) - `babel-plugin-transform-for-of`, `babel-preset-env` - [#​16011](babel/babel#16011) fix: `for of` with `iterableIsArray` and shadowing variable ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helpers`, `babel-plugin-proposal-decorators`, `babel-runtime-corejs2`, `babel-runtime-corejs3`, `babel-runtime` - [#​16144](babel/babel#16144) Set function name for decorated private non-field elements ([@​JLHwung](https://github.com/JLHwung)) - `babel-plugin-transform-typescript` - [#​16137](babel/babel#16137) Fix references to enum values with merging ([@​nicolo-ribaudo](https://github.com/nicolo-ribaudo)) ##### 🔬 Output optimization - `babel-helper-create-class-features-plugin`, `babel-plugin-transform-class-properties` - [#​16159](babel/babel#16159) Reuse computed key memoiser ([@​JLHwung](https://github.com/JLHwung)) - `babel-helpers`, `babel-plugin-proposal-decorators` - [#​16160](babel/babel#16160) Optimize decorator helper size ([@​liuxingbaoyu](https://github.com/liuxingbaoyu)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=--> Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/129 Co-authored-by: Renovate Bot <renovate@vylpes.com> Co-committed-by: Renovate Bot <renovate@vylpes.com>
In this PR we reuse the memoising expression of the computed class element key when transforming class fields. This optimizing trick can be applied to other plugins that requires memoising an expression, such as optional chaining and nullish coalescing as well.
I think eventually we should create a new helper for such purpose. For any given expression, an optimized memoising procedure can be
uid = expression
, treatuid
as the memoiser id, and re-use theuid = expression
assignmentuid
forexpression
and construct the assignmentuid = expression
Currently we only did the step 3 by
scope.maybeGenerateMemoised
, so Babel will create quite a few redundant memoisers if various transforms are applied to a given expression that needs memoising. For example, transforming a computed class accessor down to ES5 require 3 plugins: decorators, class properties and class transforms.