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 typescript@5 decorator #4122

Open
sirenkovladd opened this issue Aug 11, 2023 · 20 comments
Open

support typescript@5 decorator #4122

sirenkovladd opened this issue Aug 11, 2023 · 20 comments
Labels
bug Something isn't working bundler Something to do with the bundler transpiler parser || printer typescript Something for TypeScript

Comments

@sirenkovladd
Copy link
Contributor

What version of Bun is running?

v0.7.3

What platform is your computer?

Linux 5.19.0-50-generic x86_64 x86_64

What steps can reproduce the bug?

package.json

{
  "name": "bun-test",
  "module": "index.ts",
  "type": "commonjs",
  "author": "Sirenko Vlad",
  "devDependencies": {
    "@types/node": "^20.4.9",
    "bun-types": "latest"
  },
  "peerDependencies": {
    "typescript": "5"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "target": "ES2022",
    "outDir": "./built"
  }
}

4122.ts

function wrap<This, T>(value: T, ctx: ClassFieldDecoratorContext<This, T>) {
  console.log('Wrapping', value, ctx);
  ctx.addInitializer(function W() {
    console.log('Initialized', this, value);
  });
};

class A {
  @wrap
  public a: number = 1;
}

const a = new A();

What is the expected behavior?

❯ npx tsc && node built/4122.js         
Wrapping undefined {
  kind: 'field',
  name: 'a',
  static: false,
  private: false,
  access: { has: [Function: has], get: [Function: get], set: [Function: set] },
  addInitializer: [Function (anonymous)]
}
Initialized A {} undefined

What do you see instead?

❯ bun 4122.ts 
Wrapping {} a
1 | function wrap<This, T>(value: T, ctx: ClassFieldDecoratorContext<This, T>) {
2 |   console.log('Wrapping', value, ctx);
3 |   ctx.addInitializer(function W() {
      ^
TypeError: ctx.addInitializer is not a function. (In 'ctx.addInitializer(function W() {
    console.log("Initialized", this, value);
  })', 'ctx.addInitializer' is undefined)
      at wrap (<cut>/4122.ts:3:2)
      at <cut>/4122.ts:10:9

Additional information

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-0.html#decorators

@sirenkovladd sirenkovladd added the bug Something isn't working label Aug 11, 2023
@birkskyum
Copy link
Collaborator

birkskyum commented Aug 12, 2023

Problems with typescript 5 export type * from "" as well. (link)

I get:

error: Expected "=" but found "from"

export type * from 'my-package';

@sirenkovladd
Copy link
Contributor Author

sirenkovladd commented Aug 12, 2023

This issue is with decorator, not export/type

@Jarred-Sumner
Copy link
Collaborator

Problems with typescript 5 export type * from "" as well. (link)

I get:

error: Expected "=" but found "from"

export type * from 'my-package';

Fix for this #4125

@Electroid Electroid changed the title don't support typescript@5 decorator support typescript@5 decorator Aug 12, 2023
@robobun robobun added the typescript Something for TypeScript label Sep 9, 2023
@aleksei-baryshnikov
Copy link

Is it being planned tbd?

@DeniroBlack
Copy link

Waiting to support new typescript 5 decorators

@KaloyanGG
Copy link

I am waiting too, please fix this

@lkwr
Copy link

lkwr commented Feb 16, 2024

seems like ts 5 decorators are working, but not as they should. second parameter passed to the decorators which is the context is always undefined.

i think thats because internally they are treated as experimental decorators (that behind the "experimentalDecorators" flag) which also has the target/class/function as the first parameter.

@jeremychone
Copy link

I can confirm that when using Bun.build, the decorators are transpiled with __legacyDecorateClassTS, which I assume are the TypeScript experimental decorators.

This happens when the experimentalDecorators is not set to true, so tsc will generate the newer style.

Note: They are mostly incompatible, and in fact, the TypeScript implementation of the new decorator spec does shim the class... as I think it is required to have the right hook in the right place to follow the new spec while it is not yet implemented by JavaScript.

@Spenhouet
Copy link

Is there any workaround?

@jeremychone
Copy link

@Spenhouet, not that I know of, since Bun is generating this.

Interestingly, for my libraries, I rolled back from TS5 Decorators to TS4 experimental decorators. Here are the two main reasons:

  • TS4 Experimental decorators do not require "class .." to be shimmed, and therefore, are less intrusive on the code. I think I encountered a corner case with the TS5 Decorators shimming where statics in classes were not behaving correctly. These issues are quite hard to investigate, and often, the only solution is to forego those lost features. This might be a corner case, but I'm not sure.

  • Support for Bun.

Also, the decorators in the "can I use" matrix have no implementation plan, so we will have to do the shimming for many years to come (possibly for the next 5 years).

All things considered, while sticking with an old spec always feels odd, I think the net value of sticking with TS4 experimental decorators might be the better option. They are less intrusive in the code and offer better support.

Regardless, it would be great for Bun to follow the tsconfig.json experimentalDecorators setting and implement both, so we can revisit the decision at the user level.

@frednunesjr
Copy link

Same problem here.

TypeError: context.addInitializer is not a function.
(In 'context.addInitializer(contextInitializer)', 'context.addInitializer' is undefined)

While waiting I'm transpilling with tsc and using bun to run the generated .js files.

@pokatomnik
Copy link

pokatomnik commented May 18, 2024

Deno contributors have implemented this in January btw.
They made experimental Typescript decorators optional and modern ones are enabled by default right now.

@jeremychone
Copy link

@pokatomnik Do you know if they shim js class. Last time I use the latest decorator with tsc, it change my class ... to prototype ...

@pokatomnik
Copy link

@pokatomnik Do you know if they shim js class. Last time I use the latest decorator with tsc, it change my class ... to prototype ...

That's what typescript does. Not deno itself

@Autumnlight02
Copy link

experimentalDecorators should not be turned of! They're an old standard of that proposal, the new one is on by default!

@jeremychone
Copy link

I would also concur with @Autumnlight02 that the experimentalDecorators need to be preserved. It's quite involved to move from one to another, so preserving both is very useful.

@Jarred-Sumner
Copy link
Collaborator

Just to clear up ambiguity

Bun will support both ES decorators and TS decorators. We just haven’t gotten around to implementing this in the transpiler yet (and JSC hasn’t in the engine yet)

@WillsterJohnson
Copy link

related; accessor keyword, which was introduced as part of stage 3, also doesn't work.

49 |   accessor foo = 1;
                ^
error: Expected ";" but found "foo"
    at /path/to/file.ts

@Atulin
Copy link

Atulin commented Jul 31, 2024

Seems like #6051 is related? Just ran into an issue with accessors myself:

17 |    @state() accessor _loading: boolean;
                        ^
error: Expected ";" but found "_loading"
    at G:\MyProject\src-webcomponents\quote-box.ts:17:20
17 |    @state() accessor _loading: boolean;
                                ^
error: Expected identifier but found ":"
    at G:\MyProject\src-webcomponents\quote-box.ts:17:28
18 |    @state() accessor _quote: Quote;
                        ^
error: Expected ";" but found "_quote"
    at G:\MyProject\src-webcomponents\quote-box.ts:18:20
18 |    @state() accessor _quote: Quote;
                              ^
error: Expected identifier but found ":"
    at G:\MyProject\src-webcomponents\quote-box.ts:18:26

when running a simple build script:

import { Glob } from "bun";

const glob = new Glob("{src,src-webcomponents}/**/*.{ts,js}");

const files = [];
for await (const file of glob.scan()) {
	files.push(file);
}

const result = await Bun.build({
	entrypoints: files,
	outdir: "./.build",
	splitting: true,
	minify: true,
});

if (!result.success) {
	for (const message of result.logs) {
		console.error(message);
	}
}

@paperdave paperdave added transpiler parser || printer bundler Something to do with the bundler labels Sep 25, 2024
@andeeplus
Copy link

Anyone planning to use or implement stage 3 decorators will find this link very useful. As you can see, no one has found a bulletproof solution yet, but esbuild and babel seem to work smoothly for most cases.

https://github.com/evanw/decorator-tests

Considering that Bun's bundler API is inspired heavily by esbuild Migrating to Bun's bundler from esbuild should be relatively painless, addressing this gap would be a significant step and would definitely put Bun in a favorable position for those seeking this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bundler Something to do with the bundler transpiler parser || printer typescript Something for TypeScript
Projects
None yet
Development

No branches or pull requests