Reuse computed key memoiser

[JLHwung]

Q	A
Fixed Issues?	N.A.
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

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

1. If the expression is a uid reference, treat it as a constant binding and thus it is "static" within the scope
2. If the expression is an assignment uid = expression, treat uid as the memoiser id, and re-use the uid = expression assignment
3. Otherwise create a new uid for expression and construct the assignment uid = 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.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55969/

[nicolo-ribaudo]

Can we eventually port these changes to maybeGenerateMemoised itself?

[JLHwung]

The challenge of porting these changes to maybeGenerateMemoised is that we have to change the return structure and the compatibility issue kicks in.

Currently maybeGenerateMemoised returns an Identifier when the expression is cached and null otherwise. In our use cases, we construct the assignment from the identifier and insert the assignment somewhere.

For point 1, maybeGenerateMemoised should return whether the expression is a UID, since a UID must have been assigned to some expression somewhere, we can treat the expression as static and use cloneNode elsewhere.

For point 2, the API should return whether the expression is of the format uid = expression. In this case, the API user should decide whether they want to move the whole assignment along with other created assignment to preserve the execution ordering, and insert the uid as references elsewhere.

I am wondering if we need a broader helper to cover these steps, e.g. memoiseComputedKey or memoiseAccessorComputedKey.

[liuxingbaoyu]

Sorry, I didn't understand why hasUid is used, can you explain it? Thanks!

[JLHwung]

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.

[liuxingbaoyu]

In the future maybe we can use Set or WeakSet to achieve this.

[nicolo-ribaudo]

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 (_dec = _this = _super.call(this)). However, it is rare enough that it's not worth blocking this PR on it (feel free to improve other similar cases, but also it's ok to decide that it's not necessary :) )

[JLHwung]

@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 #p in o is moved out of the class (See also part 5 of #16117). To address this we can mix computed keys assignment with decorators:

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 _computedKey is already a "static" uid, so we don't have to memoize the key again.

Admittedly this case is not very practical, as users can just capture the brand check from the access property of the decorator context.