chore: migration to TC39 decorators + metadata simplification

[Sayan751]

📖 Description

With the certain outlook of the TC39 decorator moving to stage 4, we need to start preparing our decorators to follow this new spec.
The work of moving all the decorators in the framework to the new spec will be:

• add context to the decorator functions
• change usages of decorator in the core module into loose calls - the generated code for the new decorator is very big
• remove experimental decorator configs in tsconfig json
• tweak resources metadata related code

🎫 Issues

Closes #1180

Decorator migration guide

• Ensure that you are using the latest TypeScript version, if you are a TypeScript user.
• Remove the following 2 properties from the effective tsconfig.json: "emitDecoratorMetadata": true and "experimentalDecorators": true. This activates the native/TC39 decorator.
• Ensure that the target property is set to something lower than esnext. If this is set to esnext, then TypeScript assumes reasonably that the target runtime supports decorators natively, and hence does not transpile those.
• The decorator proposal does not support parameter decorators (yet). Hence, the following code won't work any longer.

  class MyClass {
    public constructor(
      @IBar private readonly bar: IBar,
      @IFoo foo: IFoo,
    ) {}
  }

  Instead, use the resolve function.

  import { resolve } from 'aurelia'; // also can be exported from `@aurelia/kernel`.
  class MyClass {
    private readonly bar: IBar = resolve(IBar);
    public constructor(
      // alternative#1
      foo: IFoo = resolve(IFoo)
    ) {
      // alternative#2
      const foo: IFoo = resolve(IFoo)
    }
  }

• If you are a TypeScript user, and were already using bindable coercers, it might not work after the update. Previously, this feature depended on the emission of design time type metadata. However, TypeScript does not emit this metadata any more with the standardized decorators (yet). Refer Reflect Metadata not supported for TC39 decorators microsoft/TypeScript#55788 and Expose design-time type information in TC39 decorator metadata when emitDecoratorMetadata: true microsoft/TypeScript#57533 for more information. For now, you can explicitly specify the type when declaring bindables.
• If you are using only Aurelia decorators in your code, it should be fine if the aforementioned changes are performed. In case you have developed your own decorators, then you need to migrate those by yourself. Here are a couple of resources that can be helpful:
  • JavaScript metaprogramming with the 2022-03 decorators API by Dr. Axel Rauschmayer
  • TypeScript 5.0 release notes.
  • Use the TypeScript playground to check the transpiled code to make you aware of the timings, at which different parts of your decorators are invoked.
• If you are using the Aurelia2 convention plugin, in most case it should work as it supposed to. In case it does not, please inform us.
• If you are not using TypeScript and were already using decorators somehow, consult your transpiler tool docs on how to use the new standard/native decorators.

[bigopon]

Nice work @Sayan751, we are on final steps to RC with this. I've had a few comments mostly around the API changes.

[bigopon]

Thanks for the replies, I think I agree on the initializer part, we can just let all our decorator wait until the initializer phase and call the define, so we can just deal with Symbol.metadata instead of having to drag context everywhere.

[codecov]

Codecov Report

Attention: Patch coverage is 88.24561% with 67 lines in your changes are missing coverage. Please review.

Project coverage is 88.43%. Comparing base (877a385) to head (1bb5475).

Files	Patch %	Lines
packages/runtime-html/src/bindable.ts	75.55%	11 Missing ⚠️
...kages/runtime-html/src/resources/custom-element.ts	82.14%	10 Missing ⚠️
packages/kernel/src/di.resolvers.ts	12.50%	7 Missing ⚠️
packages/kernel/src/resource.ts	25.00%	6 Missing ⚠️
packages/metadata/src/index.ts	79.16%	5 Missing ⚠️
packages/runtime/src/observation/observable.ts	86.11%	5 Missing ⚠️
packages/router/src/resources/viewport-scope.ts	20.00%	4 Missing ⚠️
packages/validation/src/rules.ts	76.47%	4 Missing ⚠️
...ges/runtime-html/src/resources/custom-attribute.ts	83.33%	3 Missing ⚠️
...ages/runtime-html/src/resources/value-converter.ts	70.00%	3 Missing ⚠️
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1932      +/-   ##
==========================================
- Coverage   88.63%   88.43%   -0.21%     
==========================================
  Files         261      262       +1     
  Lines       22862    22689     -173     
  Branches     5320     5283      -37     
==========================================
- Hits        20264    20064     -200     
- Misses       2598     2625      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Sayan751]

@bigopon it is ready for review.

[bigopon]

Nice work @Sayan751 , only a last few comments.

[bigopon]

this makes me realized it's an oversight that static $au is trying to support inheritance. It should be checking own static $au only, similar for the cached data of static au. Maybe we can add that in this PR to avoid having to do this?

[Sayan751]

Nope. The issue is slightly different. Once we define the metadata in Class[Symbol.metadata] (it is irrelevant how it is defined) on a parent class, it is inherited by the child class. And that causes the problem. From a different angle, it is a reasonable design that the child inherits the parents' metadata. If you want to override that, you can do so by using decorators, and the generated code for that will take care of it. But in the absence of a decorator, the metadata gets inherited, and that seems reasonable. If we had used decorators directly in our core code, ignoring the bundle size aspect, such gymnastic, like the code in question, would not be necessary.

[bigopon]

We can use static blocks, so that the code will stay with the definition and we won't have to deal with loose calls like this, it'll help with tree shaking I believe.
For inheritance, I'm not sure the implication of such free flow, but I don't have a way to explore the scenarios yet, so may be it'll be like this until we will have figured out something else.

[bigopon]

@Sayan751 I'll go ahead with the merge, the rest of the comments are minor non blocking, we can continue the discussion here and get them in later. Fantastic work 👍

[ghiscoding]

@Sayan751 @bigopon hey guys, it would be helpful to get more details of the migration in either the PR or the release (preferable in the release) when an impactful breaking change like this one comes into play. I'm trying hard to keep my open source project Aurelia-Slickgrid up to date with Aurelia 2 but there's breaking change at every beta without much migration info (none in this case). After looking at the PR and searching for something similar to my code, like @I18N decorator, and then I found out that I had to do the following changes

- constructor(@I18N private readonly i18n: I18N) {
+ constructor(private readonly i18n: I18N = resolve(I18N)) {

or this other line

- constructor(@newInstanceOf(IHttpClient) readonly http: IHttpClient) {
+ constructor(readonly http: IHttpClient = resolve(newInstanceOf(IHttpClient))) {

Showing these kind of changes would be really helpful to be displayed directly in the release (or at the minimum in the PR). Angular, esbuild, they all do that in their release and it's very helpful and quicker to migrate. It also seem like we need to modify our tsconfig.json as well, that is after reviewing this PR here

[Sayan751]

@ghiscoding You are absolutely right. It is my bad. I have plan to update the docs, but I am yet to do so. For now, I have updated the release notes with relevant details. Thank you for drawing our attention to this.

[ghiscoding]

Thanks for that and all the work you do on Aurelia 2 :)

[Sayan751]

Added another point related to coercing in the release notes.

[tareqimbasher]

@Sayan751 @bigopon Hi guys. I am also a maintainer of an open-source project, NetPad, that is built using Aurelia 2 and appreciate you adding some documentation on this. I rely pretty heavily on decorators for all my constructor injection and after reviewing the changes I had a few questions before I embark on the non-trivial changes to the codebase needed for this update:

1. You mention that

The decorator proposal does not support parameter decorators (yet)

Will parameter decorators eventually be supported, and thus be brought back to Au2 before release? Or is that up in the air. If the former, I'd rather hold off on upgrading to beta15 and wait for when they come back(?)

2. Migration instructions for parameter decorators is to use the resolve function. In your example the @IBar private readonly bar: IBar was moved outside of the constructor declaration. Is that a requirement because its a property while the IFoo remained as a constructor parameter since its not a class-scoped property and you used the resolve function in place?

I'd rather keep the class property in the constructor and use the resolve function there instead of moving it out, if possible.

3. You mention that

If you are using only Aurelia decorators in your code, it should be fine

Just want to confirm that built-in decorators like @bindable and @singleton need no changes at all besides the TypeScript specific changes mentioned.

4. Lastly, what about decorators applied to classes or methods. Are those affected by this as well? ie. For built-in decorators they are fine, but custom one need to be migrated as shown in linked articles?

Thank you guys for all the amazing work you do on Aurelia 2. Really love the framework and try to push it in business settings.

[ghiscoding]

@tareqimbasher on my side, all the @bindable and @singleton weren't touched at all by this new beta.15, so no worries for them. The real change is really all related to the dependency injection side of things. For reference you can take a look at my migration in this Aurelia-Slickgrid PR for Beta 15. I try to keep up to date with Au2 because it seems that every Beta version has breaking changes, so the more you wait, the more changes it will require you to do. I'd be curious to know when the Beta cycle ends? I mean RC was advertised not long ago I think!?

[tareqimbasher]

Thank you @ghiscoding for the input and the PR, will take a look. I'm upgrading from beta.7 and currently got it working up to beta.14 besides a bug I'm trying to figure out related to the changes with au-compose introduced with beta.8, but ya I'm going to take your advice and try to keep more up to date with every release. Recent updates look like we're gearing up for an RC soon, and the release cycle is pretty regular at every 2 weeks which is great.

Also great lib, Aurelia-Slickgrid, looking forward to using it soon!

[Sayan751]

@tareqimbasher

Will parameter decorators eventually be supported, and thus be brought back to Au2 before release?

That purely depends on whether and when this proposal reaches stage 3. It is out of our control to predict anything on that front. But yes, once the proposal is standardized, we will be glad to bring back parameter decorators. However, if I remember correctly, only the DI-related decorators were parameter decorators. You can easily replace those using resolve.

2. Migration instructions for parameter decorators is to use the resolve function. In your example the @ibar private readonly bar: IBar was moved outside of the constructor declaration. Is that a requirement because its a property while the IFoo remained as a constructor parameter since its not a class-scoped property and you used the resolve function in place?
   I'd rather keep the class property in the constructor and use the resolve function there instead of moving it out, if possible.

Feel free to do so. The release note merely shows both possibilities. Plus, if those are classes are meant for inheritance, avoiding ctor parameters in the base class, makes it easier (less code) for the child class. Having said that, you can very well do something like below.

import { resolve } from 'aurelia';
class MyClass {
  public constructor(
    private readonly bar: IBar = resolve(IBar),
  ) {  }
}

Just want to confirm that built-in decorators like @bindable and @singleton need no changes at all besides the TypeScript specific changes mentioned.

That is correct. Apart from the parameter decorators, every other decorator should be as it was before this change.

4. Lastly, what about decorators applied to classes or methods. Are those affected by this as well? ie. For built-in decorators they are fine, but custom one need to be migrated as shown in linked articles?

That is correct. If you have written any (experimental/TS) decorators, those needs to be migrated to the new proposal. Here is an example, how we migrated our decorators at work.

Happy Migration! ✌️

[tareqimbasher]

@Sayan751 Thank you for the detailed response!

[bigopon]

@ghiscoding @tareqimbasher

I'd be curious to know when the Beta cycle ends? I mean RC was advertised not long ago I think!?

with the completion of the decorator migration, RC is considered around the corner. We will have a few more beta versions as "buffer" to be safe, and to resolve potential issues with the new decorator implementation. The decorator proposal also getting new capabilities considered so it's not even bad for us to delay a bit.

every Beta version has breaking changes, so the more you wait, the more changes it will require you to do

Thanks for the reminder @ghiscoding , I should say that while there' breaking changes, it's not the kind of changes that rock your apps, so the benefit of having checkpoints for applications to migrate will be appreciated later (at least from the maintainers perspective, maybe they'll be cursed less), especially there' probably many apps in that category.