Expose design-time type information in TC39 decorator metadata when `emitDecoratorMetadata: true`

artberri commented 9 months ago

🔍 Search Terms

decorators, metadata, emitDecoratorMetadata, "TC39 decorators", "type information", reflect-metadata

✅ Viability Checklist

[X] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[X] This wouldn't change the runtime behavior of existing JavaScript code
[X] This could be implemented without emitting different JS based on the types of the expressions
[X] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
[X] This isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
[X] This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

I want to be able to have design-time type information at runtime after the implementation of the new TC39 decorators system.

After the implementation of the TC39 decorators in Typescript 5.0 the ability to emit design-time type information has been lost unless we keep using the experimental (legacy) decorators system.

The suggestion is to allow to use emitDecoratorMetadata: true with experimentalDecorators: false. The idea is to expose the same metadata that it was being exposed to Reflect.metadata("design:type", type) with the legacy decorators, to the new metadata system proposed by TC39 that has been implemented in Typescript 5.2.

So, instead of using the obsolete reflect-metadata like Reflect.getMetadata('design:type', target, property), we can get the same info by doing something like SomeClass[Symbol.metadata]["design:type"].

📃 Motivating Example

After the decorators change, many existing and widely used libraries will have problems implementing their behaviors without type information at runtime: type-graphql, inversify, Angular 2,... or any other library which depends on reflect-metadata.

💻 Use Cases

As explained in the "Motivating Example", there are several existing use cases that require some kind of design-time type information at runtime. My suggestion is to use more or less the same approach you used with the experimental decorators, but with the standard ones. But I will mostly agree to any other approach that makes this information available at runtime.

For now, the workaround is to keep using the experimental decorators and the reflect-metadata library (or compatible). But I think it is worth giving the standard ones the same ability to expose this info.

RyanCavanaugh commented 9 months ago

This could be implemented without emitting different JS based on the types of the expressions

Can you give some examples of how this would work while maintaining this invariant?

bcheidemann commented 7 months ago

This could be implemented without emitting different JS based on the types of the expressions

Can you give some examples of how this would work while maintaining this invariant?

@RyanCavanaugh I believe the invariant would be maintained as the feature requires emitting different JS based on the types of declarations, not expressions:

class Example {
    property: string; // This is a property declaration, not an expression
    method(param: string): string { // This is a method declaration, not an expression
        return "";
    }
}

Please let me know if I've misunderstood btw! :)

bcheidemann commented 7 months ago

I've done some investigation into how this could be implemented. As far as I can tell, this would be a fairly trivial change to implement. I was able to get it working for a few basic examples with some pretty minimal changes:

diff --git a/src/compiler/factory/emitHelpers.ts b/src/compiler/factory/emitHelpers.ts
index 0937915e75..9e6e06a1d5 100644
--- a/src/compiler/factory/emitHelpers.ts
+++ b/src/compiler/factory/emitHelpers.ts
@@ -736,7 +736,9 @@ export const metadataHelper: UnscopedEmitHelper = {
     priority: 3,
     text: `
             var __metadata = (this && this.__metadata) || function (k, v) {
-                if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
+                return function (_, c) {
+                    c.metadata[k] = v;
+                }
             };`,
 };

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
index c46e910a53..dd9f29cb1c 100644
--- a/src/compiler/program.ts
+++ b/src/compiler/program.ts
@@ -4357,13 +4357,6 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
             }
         }

-        if (
-            options.emitDecoratorMetadata &&
-            !options.experimentalDecorators
-        ) {
-            createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_without_specifying_option_1, "emitDecoratorMetadata", "experimentalDecorators");
-        }
-
         if (options.jsxFactory) {
             if (options.reactNamespace) {
                 createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, "reactNamespace", "jsxFactory");
diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts
index f7af3ceb8e..9f51a89ca7 100644
--- a/src/compiler/transformers/ts.ts
+++ b/src/compiler/transformers/ts.ts
@@ -1112,7 +1112,6 @@ export function transformTypeScript(context: TransformationContext) {
      */
     function getTypeMetadata(node: Declaration, container: ClassLikeDeclaration) {
         // Decorator metadata is not yet supported for ES decorators.
-        if (!legacyDecorators) return undefined;
         return USE_NEW_TYPE_METADATA_FORMAT ?
             getNewTypeMetadata(node, container) :
             getOldTypeMetadata(node, container);

Of course, in order to still support legacy decorators, some additional changes would be required. But these would likely also be straightforward.

bcheidemann commented 7 months ago

@RyanCavanaugh I've raised a draft PR to serve as a proposal for how this could be implemented, and to serve as a PoC. Please let me know if there's anything else I can do to help progress this issue :)

artberri commented 7 months ago

This could be implemented without emitting different JS based on the types of the expressions

Can you give some examples of how this would work while maintaining this invariant?

Aside from what @bcheidemann said, I checked this box because it won't emit different JS than the one emitted by the legacy decorators. So, if this invariant was met by the previous implementation, it would also be met by the new one.

@RyanCavanaugh, if you interpret this to mean that mimicking the previous behaviour does not preserve it, I will uncheck it, but I still think that this should be implemented to preserve decorator metadata support, as a lot of libraries depend on it.

BTW, thank you very much @bcheidemann for your support and the proposal PR.

fatcerberus commented 7 months ago

So, if this invariant [no different JS based on types] was met by the previous implementation, it would also be met by the new one.

fwiw that invariant isn't met by the legacy implementation - it falls under the grandfather clause because it was done before the "no type-directed emit" rule was instituted. This is similar to how

This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)

is violated by enum and namespace, for the same reasons.

bcheidemann commented 7 months ago

Thanks for clarifying @fatcerberus. Do you know what the thinking is on this... since it was supported for legacy decorators, is it likely to continue being supported with the new ES decorators in some capacity despite violating this invariant?

Also could you confirm how exactly the invariant is violated by this feature? It might be helpful to have some explanation of this if the feature ends up being rejected on these grounds.

bruceharrison1984 commented 4 months ago

If this is rejected, is there some alternative proposal that would emerge?

With Stage 3 metadata, the current lack of something such as design:paramtypes greatly reduces the utility of decorators for something like dependency injection as an obvious example. The absence of this information effectively relegates decorators to glorified function wrappers, and metadata into just a bag of arbitrary run-time values.

To further that point, any existing code that leverages those existing aspects would forever be stuck with reflect-metadata and experimentalDecorator, and would never be able to make the jump back to the native decorators/reflection systems.

bcheidemann commented 3 months ago

To further that point, any existing code that leverages those existing aspects would forever be stuck with reflect-metadata and experimentalDecorator, and would never be able to make the jump back to the native decorators/reflection systems.

I see this as a potentially quite a harmful split in the ecosystem. If your project uses experimentalDecorators it cannot use any packages which rely on ES decorators, and vice versa.

Worse still, a switch from experimentalDecorators to ES decorators may not be practical for many large projects which make heavy use of experimentalDecorators, meaning if TS someday drops support for them, then this could leave such projects stranded on older versions of TS.

samueldcorbin commented 1 month ago

Moreover, that split probably harms the project goals even more than just adding this feature.

It feels like this is being treated as a choice between violating that goal and not violating it.

But since this feature is in widespread use and the behavior can't be replicated without it, the actual choice seems to be between having a violation of that goal and having a violation of that goal plus an awkward, non-conforming dual-path decorator spec.

RomainLanz commented 5 days ago

Hi everyone! 👋🏻

Are there any updates on a potential migration path for dependency injection?

When using the official decorator proposal, the TypeScript compiler doesn't emit type metadata as it did with the legacy version.

function inject() {
  return function(target: any, context: any): void {
    console.log(context) // Metadata is empty
  }
}

class Service {}

@inject()
class Controller {
  constructor(private service: Service) {}
}

{
  "kind": "class",
  "name": "Controller",
  "metadata": {}
}

In this example, I would expect the metadata field to include Service when emitDecoratorMetadata is set to true. Maybe we could introduce a new flag specifically for this purpose.

This is the only missing piece needed to migrate away from reflect-metadata for many frameworks and libraries.

microsoft / TypeScript