Class field transformation is not spec compliant

[LarsDenBakker]

class Foo {
  foo = "bar";
}

Transforms into

class Foo {
  constructor() {
    this.foo = "bar";
  }
}

This is not consistent with the spec and current implementations in browsers and node. It should use defineProperty: https://github.com/tc39/proposal-class-fields#major-design-points

This is the babel output:

function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

class Foo {
  constructor() {
    _defineProperty(this, "foo", 'bar');
  }
}

https://babeljs.io/repl#?browsers=chrome%2070&build=&builtIns=false&spec=true&loose=true&code_lz=MYGwhgzhAEBiD29oG8BQ1oDNHQLzQHIAjMAJwIG5pUBfIA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Cenv&prettier=false&targets=&version=7.10.2&externalPlugins=%40babel%2Fplugin-proposal-class-properties%407.10.1

[LarsDenBakker]

Typescript has the useDefineForClassFields flag for this:

https://www.typescriptlang.org/play/index.html?useDefineForClassFields=true#code/MYGwhgzhAEBiD29oG8BQ0PQGaOgXmgHIAjMAJ0IG5UBfIA

[evanw]

The current class transform works the way it does because it follows TypeScript's transform, and because I personally wouldn't ever ship code like that in production because of the performance overhead. I can definitely see adding opt-in support for this via the useDefineForClassFields flag though.

[LarsDenBakker]

The new class field spec has pros and cons, and not everybody is in favor of it.

The difference between the spec and the common practice until now is going to be a challenge for the community. I'm worried about people having their code behave differently between when they run their code as is, and when they run a build on it. It may lead to a lot of subtle bugs.

[bathos]

The difference between the spec and the common practice until now [...]

Babel follows the proposed JS semantics, not the TS semantics. The use of static and instance fields via Babel is especially popular in the React world. Chrome and Firefox have been shipping this unflagged for a while. The misalignment is unfortunate, but it seems to be a TS-JS thing, not a common practice vs specified behavior thing. (Edit: having just re-read the thread, I realize you probably meant ”common practice [in TS]” anyway so nm.)

(Had a related post here yesterday which is partly redundant w/ what’s been discussed in this thread, but didn’t see this thread till today.)

[evanw]

I just released esbuild version 0.5.9 with a fix for this. Although the default behavior hasn't changed, you can now enable strict compliance with the spec using the --strict flag. Spec compliant behavior is also enabled if the useDefineForClassFields flag is true in tsconfig.json.

[LarsDenBakker]

Thanks a lot!

[eligrey]

@evanw I have compilerOptions.useDefineForClassFields set to true, but I can't get esbuild to transpile this code correctly no matter what config I use.

esbuild is moving these fields into the constructor without defineProperty semantics, which is invalid and throws an error:

class BrandedEventTarget extends EventTarget {
  [Symbol.toStringTag] = 'my brand';
};
console.log(new BrandedEventTarget());

transpiled:

var _a8, _b2;
var BrandedEventTarget = (_b2 = class extends EventTarget {
  constructor() {
    super(...arguments);
    this[_a8] = MY_BRAND;
  }
}, _a8 = toStringTagSymbol, _b2);

[bathos]

@eligrey the problem is not that the field has been moved to the constructor (semantically, it's happening in the right place), it's that there is an inherited @@toStringTag property which is an unwritable data property and the transformation is using [[Set]] instead of [[DefineOwnProperty]] semantics. The former will throw if an unwritable property exists in the prototype chain.

[eligrey]

@bathos I cannot seem to get useDefineForClassFields to work with this or maybe esbuild isn't using my tsconfig. How would I get esbuild to properly compile my BrandedEventTarget example?

In the meantime I've just switched to manually calling Object.defineProperty(this, Symbol.toStringTag, ...) in the constructor myself.

[bathos]

It may be an esbuild issue, I'm not sure (I haven't used esbuild with TS before myself), but for what it's worth usually you'd want that on the prototype - like if you're doing this for a high-fidelity polyfill, it'd be:

Object.defineProperty(BrandedEventTarget.prototype, Symbol.toStringTag, {
  configurable: true,
  value: 'my brand'
});

...though I'd note this is not what a "brand" is (a real brand would need to be realized with weak collection semantics).