Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support emitting typescript decorator metadata #257

Closed
monoclex opened this issue Jul 14, 2020 · 19 comments
Closed

Support emitting typescript decorator metadata #257

monoclex opened this issue Jul 14, 2020 · 19 comments

Comments

@monoclex
Copy link

For projects such as TypeORM, it is necessary for decorator metadata to get emitted. For example, let's take TypeORM's typescript-example project. Running the project normally yields the following output.

git clone https://github.com/typeorm/typescript-example --depth 1
cd typescript-example
yarn && yarn start

image

Attempting to use ESBuild successfully compiles it with warnings, but fails with a runtime error.

esbuild --bundle --outfile=index.js --target=esnext --platform=node --format=cjs --strict .\src\index.ts

image

@evanw
Copy link
Owner

evanw commented Jul 14, 2020

The emitDecoratorMetadata flag is intentionally not supported. It relies on running the TypeScript type checker, which relies on running the TypeScript compiler, which is really slow. Given that esbuild's primary purpose is speed, I will not be integrating the TypeScript compiler into esbuild. I will also not be rewriting the TypeScript type checker in Go since that would be a massive ongoing maintenance burden. It's possible that you could write an esbuild plugin for this when esbuild's plugin API materializes (see #111) but that would likely eliminate most of the speed advantage of using esbuild in the first place. You're probably better off using another tool instead of esbuild if you need to do this.

@monoclex
Copy link
Author

monoclex commented Jul 14, 2020

What do you think about adding some way to short circuit the type checking process, such as an @esbuild-type [number/string/object/boolean/function] annotation? That could be a solution for this situation specifically at least, but I'm not sure about the versatility of the idea at a larger scope. It's certainly not an ideal solution and is at best a hack, but if it could be useful in other areas that'd be a great side effect.

EDIT: a little explanation on how an @esbuild-type would help implementing this specifically:
The following TS class

import { Column, Entity, PrimaryGeneratedColumn } from "typeorm";

@Entity()
export class TestModel {

  @PrimaryGeneratedColumn()
  id!: number;

  @Column()
  data!: string;

  @Column()
  complicated!: SomethingComplicated<string, number, Record<string, unknown>>

  @Column()
  a!: boolean;

  @Column()
  a2!: Function;
}

interface SomethingComplicated<T, U, V> {
  a: T;
  u: SomethingComplicated<T, U, V>[];
  v: V[]
}

will be transpiled to the following JS

// ...
const typeorm_1 = require("typeorm");
let TestModel = class TestModel {
};
__decorate([
    typeorm_1.PrimaryGeneratedColumn(),
    __metadata("design:type", Number)
], TestModel.prototype, "id", void 0);
__decorate([
    typeorm_1.Column(),
    __metadata("design:type", String)
], TestModel.prototype, "data", void 0);
__decorate([
    typeorm_1.Column(),
    __metadata("design:type", Object)
], TestModel.prototype, "complicated", void 0);
__decorate([
    typeorm_1.Column(),
    __metadata("design:type", Boolean)
], TestModel.prototype, "a", void 0);
__decorate([
    typeorm_1.Column(),
    __metadata("design:type", Function)
], TestModel.prototype, "a2", void 0);
// ...

thus, implementing specifically those 5 types would solve it for only reflect-metadata stuff, but I'm unaware of any other scenarios where it'd be beneficial to everyone at large.

@evanw
Copy link
Owner

evanw commented Jul 14, 2020

That annotation seems too much of a special-case to put into esbuild core to me. Besides, if you want to be able to do this:

class Foo {
  @decorator
  // @esbuild-type number
  prop: number
}

couldn't you just do something like this instead?

class Foo {
  @decorator
  @esbuildType(Number)
  prop: number
}

where esbuildType is just a function that forwards to the TypeScript metadata function:

let esbuildType = t => __metadata("design:type", t)

I think that would do what you're trying to accomplish without needing to extend esbuild at all. In fact it's even shorter than the comment annotation form.

I know implementing just those types would be part of the feature, but I don't think it's a good idea to claim to support it and then work differently in subtle ways that silently break things. Even those primitive types can be propagated through arbitrary aliases and type expressions in the type system and can probably be imported from other files. Getting all of that correct means re-implementing the TypeScript type checker.

@monoclex
Copy link
Author

Alright, that's completely understandable. Thanks for taking the time to run through the idea 👍

@axe-me
Copy link

axe-me commented Feb 12, 2021

I saw swc actually support emitDecoratorMetadata. This is how it did it: https://github.com/swc-project/swc/pull/939/files
Would esbuild so something similar?
lots of backend packages are built on top of this flag. it's kinda annoying not able to use esbuild to improve backend DX.

Thanks.

@evanw
Copy link
Owner

evanw commented Feb 12, 2021

This is in scope for swc because they are trying to replicate the whole TypeScript type system in swc. Putting a type system into esbuild is firmly out of scope for esbuild. If you need this feature you should use swc/spack instead (or maybe use swc as a plugin for esbuild?).

@shadowtime2000
Copy link

@evanw I am pretty sure that the SWC project to replicate tsc is a closed source one, while the open source parts of SWC support emitDecoratorMetadata so that may not be the case, but I am not 100% sure.

@evanw
Copy link
Owner

evanw commented Feb 19, 2021

Ah weird. Ok well I guess never mind. I missed that part. Looks like this is the relevant comment: swc-project/swc#571 (comment)

But it’s not open sourced, and I’m not going to open source it.

@shadowtime2000
Copy link

@evanw Do you think it may still be possible considering how the parts of SWC which act just like Babel are able to do it?

@thomaschaaf
Copy link

I'm currently trying to write an esbuild plugin which will try to build with esbuild and compile only the files containing docorators with tsc again. This should result in faster build times as usually not all files use decorators. See #915 (comment)

@thomaschaaf
Copy link

thomaschaaf commented Mar 6, 2021

I have released the following plugin: esbuild-plugin-tsc which allows you to now use emitDecoratorMetadata with esbuild.

@vikasgarghb
Copy link

@evanw Is there any workaround to this issue?
@SirJosh3917 What solution did you go for?

@evanw
Copy link
Owner

evanw commented Mar 10, 2021

@evanw Is there any workaround to this issue?

One workaround is in the comment immediately preceding yours: you can write an esbuild plugin that runs the TypeScript transpiler on source code files containing decorators before passing the files to esbuild.

@monoclex
Copy link
Author

@SirJosh3917 What solution did you go for?
I opted to not use esbuild, since I needed it.

rxliuli added a commit to rxliuli/joplin-utils that referenced this issue May 19, 2021
@robbie-c
Copy link

robbie-c commented Jan 28, 2023

Just as an experiment I wanted to test how sophisticated the type inference is when creating this metadata. TLDR it doesn't actually do much resolution of complex types at all, and doing something good enough for most use cases could have ok performance.

Input file (an example TypeORM table)

// snipped imports

@Entity("some_table")
class SomeTable extends BaseEntity {
  @PrimaryGeneratedColumn("uuid")
  id: string;

  @Column()
  num: number;

  @Column()
  something_hard: Pick<{ a: string; b: number }, "a">["a"]; // string

  @OneToOne(() => DBDeviceUUID)
  lazy_relation: Promise<DBDeviceUUID>;

  @OneToOne(() => DBDeviceUUID)
  eager_relation: DBDeviceUUID;

  @Column()
  a_set: Set<string>;

  @Column()
  a_map: Map<string, string>;

  @Column()
  a_bigint: BigInteger;
}

const x: SomeTable["something_hard"] =
  "just proving that this is just a string";

output file

// snipped defintions of metadata and decorate, and snipped requires

let SomeTable = class SomeTable extends typeorm_1.BaseEntity {
};
__decorate([
    (0, typeorm_1.PrimaryGeneratedColumn)("uuid"),
    __metadata("design:type", String)
], SomeTable.prototype, "id", void 0);
__decorate([
    (0, typeorm_1.Column)(),
    __metadata("design:type", Number)
], SomeTable.prototype, "num", void 0);
__decorate([
    (0, typeorm_1.Column)(),
    __metadata("design:type", Object)
], SomeTable.prototype, "something_hard", void 0);
__decorate([
    (0, typeorm_1.OneToOne)(() => DeviceUUID_1.DBDeviceUUID),
    __metadata("design:type", Promise)
], SomeTable.prototype, "lazy_relation", void 0);
__decorate([
    (0, typeorm_1.OneToOne)(() => DeviceUUID_1.DBDeviceUUID),
    __metadata("design:type", DeviceUUID_1.DBDeviceUUID)
], SomeTable.prototype, "eager_relation", void 0);
__decorate([
    (0, typeorm_1.Column)(),
    __metadata("design:type", Set)
], SomeTable.prototype, "a_set", void 0);
__decorate([
    (0, typeorm_1.Column)(),
    __metadata("design:type", Map)
], SomeTable.prototype, "a_map", void 0);
__decorate([
    (0, typeorm_1.Column)(),
    __metadata("design:type", Object)
], SomeTable.prototype, "a_bigint", void 0);
SomeTable = __decorate([
    (0, typeorm_1.Entity)("some_table")
], SomeTable);
const x = "just proving that this is just a string";

Just to put this in a neater format

type design:type
string String
number Number
Pick<{ a: string; b: number }, "a">["a"] (string) Object
Promise<DBDeviceUUID> Promise
DBDeviceUUID (a different class) DBDeviceUUID
Set Set
Map Map
BigInteger Object

I was actually surprised that it didn't get something_hard. My personal opinion is that if tsc can't do this, then any other implementation of reflect doesn't need to either. This would mean that instead of checking whether e.g. a type eventually resolves to string, it could just check whether it's literally the characters string.

@segevfiner
Copy link

@evanw swc supports this, and I doubt they run the full TypeScript compiler, it is likely the actual transform is simpler then you think and doesn't really require complex type resolution. tsup runs swc as a workaround for this not being supported in esbuild, but it would obviously be nicer if this was supported natively.

@evanw
Copy link
Owner

evanw commented Jan 24, 2024

I tried looking into the current rules. It looks like they are here. These rules imply that:

  1. Type syntax would need to be parsed (currently esbuild treats types as whitespace)
  2. Type identifiers need to be resolved by a type checker (e.g. even declare types)
  3. Post-resolution type attributes must be inspected (e.g. is the type numeric, does the interface have a call signature)

Here is an example:

// foo.ts
declare let dec: any
import type { ns } from './bar'
class Foo {
  @dec fn(e: ns.E): ns.Fn { throw 0 }
}
// bar.ts
export namespace ns {
  export declare enum E {}
  export interface Fn { x(): void }
  export interface Fn extends Base {}
  type Base = [1] | [false] extends Array<number> ? Y : Call
  interface Y { y(): void }
  interface Call {
    (): void // This makes the return type "Function"
  }
}

You're supposed to get this:

class Foo {
    fn(e) { throw 0; }
}
__decorate([
    dec,
    __metadata("design:type", Function),
    __metadata("design:paramtypes", [Number]),
    __metadata("design:returntype", Function)
], Foo.prototype, "fn", null);

The paramtypes include Number because ns.E is resolved to an enum type. This involves doing cross-file type resolution and can't be done at run-time since E is just a type declaration and isn't actually a value. For a more complex example, the returntype is Function instead of Object because ns.Fn (after being merged) extends from Base, and Base has type Call because while [1] extends Array<number>, [false] does not extend Array<number> so the union [1] | [false] does not extend Array<number>, and Call has a call signature.

Obviously it's possible write a type checker for all of this (tsc is a proof of that). But it still means writing a type checker for a Turing-complete type system. I agree that it would be nice if esbuild contained such a type checker. Then we wouldn't need tsc anymore! But then I'd have to write and maintain a TypeScript-compatible type checker, which is a huge undertaking and is not something I want to do (thus why I consider this to be out of scope).

Obviously it's also possible to write a simpler type checker that ignores the complexity of TypeScript and checks a different type language instead. That appears to be the approach swc has taken. However, I don't want esbuild to take that approach. That involves signing up for a constant stream of issues (as people would rightly expect esbuild's type system to behave like TypeScript instead of esbuildScript) and it wouldn't be fair to users who would have to deal with subtle and unexpected bugs.

@segevfiner
Copy link

Thanks for the detailed write up! Guess we can hope stuff will move away from using due to the newer stable decorators...

@segevfiner
Copy link

segevfiner commented Jan 25, 2024

For now, guess I'll just use tsup with it's support of using swc to transpile decorator metadata, including it's caveats of course...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants