oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.09k stars 2.67k forks source link

Reflect-metadata doesn't work for tsyringe #4677

Open i-void opened 1 year ago

i-void commented 1 year ago

What version of Bun is running?

1.0.0+822a00c4d508b54f650933a73ca5f4a3af9a7983

What platform is your computer?

Linux 6.2.0-32-generic x86_64 x86_64

What steps can reproduce the bug?

@singleton() class A { constructor() { console.log('A') } }

container.resolve(A)

- modify package.json like this

{ "name": "simplest", "module": "index.ts", "type": "module", "scripts": { "start": "bun ./index.ts" }, "devDependencies": { "bun-types": "latest" }, "peerDependencies": { "typescript": "^5.0.0" }, "dependencies": { "reflect-metadata": "^0.1.13", "tsyringe": "^4.8.0" } }

- modify tsconfig like this

{ "compilerOptions": { "lib": ["ESNext"], "module": "esnext", "target": "esnext", "moduleResolution": "bundler", "moduleDetection": "force", "allowImportingTsExtensions": true, "noEmit": true, "composite": true, "strict": true, "downlevelIteration": true, "skipLibCheck": true, "jsx": "preserve", "allowSyntheticDefaultImports": true, "forceConsistentCasingInFileNames": true, "allowJs": true, "emitDecoratorMetadata": true, "experimentalDecorators": true, "types": [ "bun-types" // add Bun global ] } }

- run `bun start`

### What is the expected behavior?

It should print `A` to the console.

### What do you see instead?

$ bun ./index.ts 1 | "use strict"; 2 | Object.defineProperty(exports, "__esModule", { value: true }); 3 | const tslib_1 = require("tslib"); 4 | if (typeof Reflect === "undefined" || !Reflect.getMetadata) { 5 | throw new Error(tsyringe requires a reflect polyfill. Please add 'import "reflect-metadata"' to the top of your entry point.); ^ error: tsyringe requires a reflect polyfill. Please add 'import "reflect-metadata"' to the top of your entry point. at /home/cm/Programming/bun/simplest/node_modules/tsyringe/dist/cjs/index.js:5:10 at globalThis (/home/cm/Programming/bun/simplest/node_modules/tsyringe/dist/cjs/index.js:15:126) error: script "start" exited with code 1 (SIGHUP)



### Additional information

When I use 
`const { container, singleton } = await import('tsyringe')` instead of direct import. It works.
m-salman-afzal commented 1 year ago

Same here, was trying to just run the node project in bun to see how it works, but stuck at this.

alexbechmann commented 1 year ago

Same, this is a blocker for me!

Murzbul commented 1 year ago

Same here!

woehrl01 commented 1 year ago

Fyi, the latest version where this worked, was bun 0.6.11 (at least the application could execute without failure)

woehrl01 commented 1 year ago

@i-void I found the rootcause of this. The problem is that the .cjs module gets imported instead of the ejs.

It looks like that the resolution does not work anymore as documented here

workaround is to modify the tsyringe's package.json to either:

or:

"exports": {
    "require": "./dist/cjs/index.js",
    "import": "./dist/esm2015/index.js"
  },

patch-package is your friend.

diff --git a/node_modules/tsyringe/package.json b/node_modules/tsyringe/package.json
index 8a7ebb7..f653a10 100644
--- a/node_modules/tsyringe/package.json
+++ b/node_modules/tsyringe/package.json
@@ -2,7 +2,11 @@
   "name": "tsyringe",
   "version": "4.8.0",
   "description": "Lightweight dependency injection container for JavaScript/TypeScript",
-  "main": "dist/cjs/index.js",
+  "exports": {
+    "require": "./dist/cjs/index.js",
+    "import": "./dist/esm2015/index.js"
+  },
+  "main": "./dist/cjs/index.js",
   "module": "./dist/esm5/index.js",
   "es2015": "./dist/esm2015/index.js",
   "typings": "./dist/typings/index.d.ts",

Edit: just found that it's an explicit breaking change in 0.6.12

PR for tsyringe: https://github.com/microsoft/tsyringe/pull/232

alexbechmann commented 1 year ago

Nice @woehrl01 !

With that fix I am able to import, however tsyringe still doesn't work as there is seemingly no metadata emitted for the classes?

error: TypeInfo not known for "MyClass"
woehrl01 commented 1 year ago

@alexbechmann yes, that's right. But the underlying issue for that is, that bun only does transpiling. I don't have the github issue at hand, but there was a clear statement that will likely not be implemented in the near future, because of the complexity.

Edit: I did some research, and it looks like that there is currently a branch by @dylan-conway open, to add this. So maybe this come in the near future. 😊

i-void commented 1 year ago

my workaround is to use https://www.npmjs.com/package/core-js for reflect. But for some libraries like typegraphql its reflect implementation is missing some methods. So I use 2 libraries together, core-js initialize the Reflect object and reflect-metada does the remainin job.

import "core-js";
import "reflect-metadata";

This bug is still annoying though just wanted to share this as a workaround.

alexbechmann commented 1 year ago

I have an example repo with tsyringe working using a fixed version of esbuild-plugin-tsc: https://github.com/alexbechmann/bun-metadata-workaround for anybody needing a quick workaround until the real issue is resolved.

It works because the plugin compiles any file with decorators with tsc and therefore emitting the correct metadata.

alexbechmann commented 11 months ago

Tsyringe works with latest canary for me, without any plugins

bun upgrade --canary

Zikoat commented 11 months ago

This should be fixed in Bun v1.0.3 🎉🎉🎉 See the blog post and https://github.com/oven-sh/bun/issues/4575 This issue can be closed @i-void

jonnysamps commented 11 months ago

Using bun 1.0.3 and this is still happening for me.

alekseibaryshnikov commented 10 months ago

Still facing this issue on 1.0.11. 😢

SamuelGaona commented 9 months ago

Same problem on v1.0.13

tried:

root tsconfig.json --ts-config-override (has no effect)

JMDowns commented 9 months ago

There's a nice way to fix this as a workaround:

Make a bunfig.toml file in your root directory, and make the contents something like preload = ["./reflect-metadata-import.ts"]

Inside of 'reflect-metadata-import.ts', the content should be import 'reflect-metadata';

Running bun start after doing this gives me 'A'.

BrennanColberg commented 8 months ago

Still broken for me—I could replicate the exact bug as reported—on Bun v1.0.20. Can confirm that @JMDowns' patch works, though this is obviously suboptimal.

dylan-conway commented 8 months ago

This bug is happening because bun is evaluating modules in the wrong order sometimes. minimal repro: index.mjs:

import "./foo.cjs"
import bar from "usesGlobalFoo.cjs"

foo.cjs:

globalThis.foo = "foo";

usesGlobalFoo.cjs:

console.log(globalThis.foo);
module.exports = "bar";

Running this with node will print "foo" from the console.log in usesGlobalFoo.cjs. bun will print undefined because foo.cjs has not been evaluated yet.

This breaks tsyringe because import "reflect-metadata" will not modify global Reflect before the reflect metadata methods are used

dilbarov commented 8 months ago

Confirmed, if I run bun --hot index.ts, make minimal edits to the code, bun rebuilds the build and the error is not reproduced

fccoelho7 commented 3 months ago

any update on that?

etorralba commented 2 months ago

It works when importing reflect-metadata at the Top of Your Entry Point File. In other words, you need to add import 'reflect-metadata'; at the very top of your main entry point file (usually src/index.ts or src/app.ts).

oblx commented 2 months ago

I had a try today after upgrading to latest 1.1.18, import 'reflect-metadata' at the top of my entrypoint src/index.ts and still buggy.

Luckily the workaround provided by @JMDowns fixed this issue 👌

oblx commented 1 month ago

@TheAyes my current working setup using bun 1.1.21 and based on the previous answer :

./bunfig.toml

preload = ["./src/config/reflectMetadata.ts"]
sourcemap = "external"
tsconfig = "tsconfig.json"

./src/config/reflectMetadata.ts

import 'reflect-metadata';

I used to have import 'reflect-metadata'; in index.ts but it looks fine without. In both run and bundle commands.

Edit : my deepest apologies, there are no tests in this project 🤦‍♂️

lipex360x commented 2 weeks ago

@TheAyes my current working setup using bun 1.1.21 and based on the previous answer :

./bunfig.toml

preload = ["./src/config/reflectMetadata.ts"]
sourcemap = "external"
tsconfig = "tsconfig.json"

./src/config/reflectMetadata.ts

import 'reflect-metadata';

I used to have import 'reflect-metadata'; in index.ts but it looks fine without. In both run and bundle commands.

Edit : my deepest apologies, there are no tests in this project 🤦‍♂️

For some reason that I really have no idea about, after a lot of head scratching I managed to "solve" the problem of reflect metadata in tests

in the root of my tests directory, what I did was create a file app.spec.ts with the following content

import { beforeAll } from 'bun:test'

import { app } from '@/app'

beforeAll(() => app.mount('/', () => null))

It is very important that this file is in the ROOT of the directory and not inside other subdirectories for it to work.

Reason: Once again... I have no idea

image

app.ts content (using core-js and tsyringe for dependency injection )

import '@/infra/containers'
import 'core-js'

import { Hono } from 'hono'
import { logger } from 'hono/logger'

import { carRoutes } from './infra/routes'

const app = new Hono()
app.use(logger())

app.route('/car', carRoutes)

export { app }
image