Skip to content

Commit

Permalink
update handling of decorators to support emitting ES2022+ (#3614)
Browse files Browse the repository at this point in the history
This updates our handling of Stencil-defined decorators to enable
Stencil users to set an ES2022+ `target` in their `tsconfig.json`.

This requires a code change on our end because as of ES2022 class fields
are now a standard part of ECMAScript, so the TypeScript compiler will
emit a class field when it is present in the source, so this:

```ts
class Foo {
    field: string = "blah";
}
```

will compile to something like

```js
class Foo {
    field = "blah";
}
```

Previously it compiled to something like this:

```js
class Foo {
    constructor() {
        this.field = "blah";
    }
}
```

Now why is that a problem for us?

When a class field is decorated with a Stencil-defined decorator, we
rely on defining our own setters and getters (using
`Object.defineProperty`) to implement the behavior we want.
Unfortunately, the behavior defined in the ECMAScript standard for class
fields is incompatible with using manually-defined getters and setters,
so we ultimately have to convert some decorated class fields to be
initialized in the constructor instead.

In ES2022+ if we try to use `Object.defineProperty` on this class's
prototype in order to define a `set` and `get` function for a property
`foo` which is declared using the class field syntax it will not
override the default behavior of the instance field `foo`, so doing
something like the following:

```ts
class MyClass {
    foo = "bar";
}

Object.defineProperty(MyClass.prototype, "foo", {
  get() {
    return "Foo is: " + this.foo
  }
});
```

and then calling `myClassInstance.foo` will _not_ return `"Foo is: bar"` but
just `"bar"`. This is because the standard ECMAScript behavior is now to use
the internals of `Object.defineProperty` on a class instance to instantiate
fields, and that call at instantiation-time overrides what's set on the
prototype. For details, see the accepted ECMAScript proposal for this
behavior:

https://github.com/tc39/proposal-class-fields#public-fields-created-with-objectdefineproperty

Why is this important? With `target` set to an ECMAScript version prior to
ES2022 TypeScript by default would emit a class which instantiated the field
in its constructor, something like this:

```ts
class CompiledMyClass {
  constructor() {
    this.foo = "bar"
  }
}
```

This plays nicely with later using `Object.defineProperty` on the
prototype to define getters and setters, or simply with defining them
right on the class (see the code in `proxyComponent`,
`proxyCustomElement`, and friends, where we do just this).

However, with a `target` of ES2022 or higher (e.g. `ESNext`) default
behavior for TypeScript is instead to emit code like this:

```ts
class CompiledMyClass {
  foo = "bar"
}
```

This output is more correct because the compiled code 1) more closely
resembles the TypeScript source and 2) is using standard JS syntax instead
of desugaring it. There is an announcement in the release notes for
TypeScript v3.7 which explains some helpful background about the change,
and about the `useDefineForClassFields` TypeScript option which lets you
opt-in to the old output:

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier

For our use-case, however, the ES2022+ behavior doesn't work, since we
need to be able to define getters and setters on these fields. We could
require that the TypeScript configuration used for Stencil have the
`useDefineForClassFields` setting set to `false`, but that would have
the undesirable side-effect that class fields which are _not_ decorated
with a Stencil decorator for which we require getters and setters
(`@State`, `@Prop`) would _also_ be instantiated in the constructor.

So instead, we take matters into our own hands. When we encounter a
class field which is decorated with a Stencil decorator we remove it
from the class and add a statement to the constructor to instantiate it
with the correct default value.

This commit makes the change to do that for fields decorated with
`@State` and `@Prop`.
  • Loading branch information
alicewriteswrongs authored Oct 5, 2022
1 parent 2db4f4d commit f977830
Show file tree
Hide file tree
Showing 4 changed files with 538 additions and 33 deletions.
Loading

0 comments on commit f977830

Please sign in to comment.