-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Reflect-metadata doesn't work for tsyringe #4677
Comments
Same here, was trying to just run the node project in bun to see how it works, but stuck at this. |
Same, this is a blocker for me! |
Same here! |
Fyi, the latest version where this worked, was bun 0.6.11 (at least the application could execute without failure) |
@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: microsoft/tsyringe#232 |
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?
|
@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. 😊 |
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.
This bug is still annoying though just wanted to share this as a workaround. |
I have an example repo with tsyringe working using a fixed version of It works because the plugin compiles any file with decorators with tsc and therefore emitting the correct metadata. |
Tsyringe works with latest canary for me, without any plugins bun upgrade --canary |
Using bun 1.0.3 and this is still happening for me. |
Still facing this issue on 1.0.11. 😢 |
Same problem on v1.0.13 tried: root tsconfig.json |
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 Inside of 'reflect-metadata-import.ts', the content should be Running bun start after doing this gives me 'A'. |
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. |
This bug is happening because bun is evaluating modules in the wrong order sometimes. minimal repro: 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 This breaks tsyringe because |
Confirmed, if I run |
any update on that? |
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). |
I had a try today after upgrading to latest 1.1.18, Luckily the workaround provided by @JMDowns fixed this issue 👌 |
Has anyone figured out how to use it for tests yet? I've applied the preload workaround but it seems to not be gripping during tests as they just hang indefinetly (just like my application back when I didn't use the workaround) |
@TheAyes my current working setup using bun 1.1.21 and based on the previous answer :
I used to have 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
|
While i appreciate your efforts. I don't really have a dedicated tests directory. I always couple them next to the files they're most relevant for. :/ Apparently i might need to wait for a few more updates ^-^ |
Any news on this? |
Did you try the fix that uses |
nop let me try this, I thought bun is drop in replacement of node and no additional configs are needed)) I think the orders of imports should be respected, that can intoroduce bugs with other libs as well I guess |
To special cases, special fixes 😄 |
Yes... I solved using Lib Core-JS instead of Reflect-Metadata. just import into your file as you would with Reflect-Metadata. |
@lipex360x how i did that? can you explain me more? |
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?
bun start
What is the expected behavior?
It should print
A
to the console.What do you see instead?
Additional information
When I use
const { container, singleton } = await import('tsyringe')
instead of direct import. It works.The text was updated successfully, but these errors were encountered: