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

fix: allow propertyKey to be undefined #226

Merged
merged 2 commits into from Apr 17, 2023

Conversation

JorgeLNJunior
Copy link
Contributor

Description

Typescript 5 fixed a bug in the decorator parameter type check that was causing decorators from tsyringe to fail to build. This fix allows for the propertyKey parameter to be undefined.

I'm not an experienced programmer, so if there's something wrong, please let me know.

Fixes: #221

@m-salman-afzal
Copy link

any updates on this? Only this is the hinderence while migrating to TS 5😢😢

@k2tomasz
Copy link

asked someone from MS to look into it. hopefully they will approve this soon.

@koloboid
Copy link

Guys, this needs to be merged asap, the project becomes unusable.

@vajahath
Copy link

We might want to tag someone from MS I guess..

@fknop
Copy link

fknop commented Apr 16, 2023

@Xapphire13 Hello!
I'm tagging you as you are the main contributor to this project. Is this project still maintained by microsoft? I can see on your Github profile that you no longer work for them. Would there be someone to take over the repository?

@MeltingMosaic
Copy link
Collaborator

MeltingMosaic commented Apr 17, 2023

LGTM

I don't love the idea of propertyKey being undefined, but I'm OK with it as a stop gap until we can get a proper TS 5 version.

@MeltingMosaic MeltingMosaic merged commit 3f25002 into microsoft:master Apr 17, 2023
1 check passed
@fknop
Copy link

fknop commented Apr 17, 2023

Thanks @MeltingMosaic ! Do you know when a new version will be released? :)

@rghermosa
Copy link

@MeltingMosaic Do you know when will this be released?
The library is unusable for the newer versions of TS and this supposes a blocker for many people who want to use this library as well

@m-salman-afzal
Copy link

@MeltingMosaic Do you know when will this be released? The library is unusable for the newer versions of TS and this supposes a blocker for many people who want to use this library as well

still waiting⌚

@fknop
Copy link

fknop commented Apr 30, 2023

For people interested, in the meantime I'm using a patch using patch-package, it's not ideal but it allowed us to upgrade to TS 5.0.

diff --git a/node_modules/tsyringe/dist/typings/decorators/inject-all-with-transform.d.ts b/node_modules/tsyringe/dist/typings/decorators/inject-all-with-transform.d.ts
index 196dfac..1a5764e 100644
--- a/node_modules/tsyringe/dist/typings/decorators/inject-all-with-transform.d.ts
+++ b/node_modules/tsyringe/dist/typings/decorators/inject-all-with-transform.d.ts
@@ -5,5 +5,5 @@ import Transform from "../types/transform";
  *
  * @return {Function} The parameter decorator
  */
-declare function injectAllWithTransform(token: InjectionToken<any>, transformer: InjectionToken<Transform<[any], any>>, ...args: any[]): (target: any, propertyKey: string | symbol, parameterIndex: number) => any;
+declare function injectAllWithTransform(token: InjectionToken<any>, transformer: InjectionToken<Transform<[any], any>>, ...args: any[]): (target: any, propertyKey: string | symbol | undefined, parameterIndex: number) => any;
 export default injectAllWithTransform;
diff --git a/node_modules/tsyringe/dist/typings/decorators/inject-all.d.ts b/node_modules/tsyringe/dist/typings/decorators/inject-all.d.ts
index 2bd7da4..3716ef5 100644
--- a/node_modules/tsyringe/dist/typings/decorators/inject-all.d.ts
+++ b/node_modules/tsyringe/dist/typings/decorators/inject-all.d.ts
@@ -4,5 +4,5 @@ import InjectionToken from "../providers/injection-token";
  *
  * @return {Function} The parameter decorator
  */
-declare function injectAll(token: InjectionToken<any>): (target: any, propertyKey: string | symbol, parameterIndex: number) => any;
+declare function injectAll(token: InjectionToken<any>): (target: any, propertyKey: string | symbol | undefined, parameterIndex: number) => any;
 export default injectAll;
diff --git a/node_modules/tsyringe/dist/typings/decorators/inject-with-transform.d.ts b/node_modules/tsyringe/dist/typings/decorators/inject-with-transform.d.ts
index 05f87d7..47ba138 100644
--- a/node_modules/tsyringe/dist/typings/decorators/inject-with-transform.d.ts
+++ b/node_modules/tsyringe/dist/typings/decorators/inject-with-transform.d.ts
@@ -7,5 +7,5 @@ import Transform from "../types/transform";
  * @param args Arguments to be passed to the transform method on the transformer
  * @returns The parameter decorator
  */
-declare function injectWithTransform(token: InjectionToken<any>, transformer: InjectionToken<Transform<any, any>>, ...args: any[]): (target: any, propertyKey: string | symbol, parameterIndex: number) => any;
+declare function injectWithTransform(token: InjectionToken<any>, transformer: InjectionToken<Transform<any, any>>, ...args: any[]): (target: any, propertyKey: string | symbol | undefined, parameterIndex: number) => any;
 export default injectWithTransform;
diff --git a/node_modules/tsyringe/dist/typings/decorators/inject.d.ts b/node_modules/tsyringe/dist/typings/decorators/inject.d.ts
index 5d48d79..867384c 100644
--- a/node_modules/tsyringe/dist/typings/decorators/inject.d.ts
+++ b/node_modules/tsyringe/dist/typings/decorators/inject.d.ts
@@ -4,5 +4,5 @@ import InjectionToken from "../providers/injection-token";
  *
  * @return {Function} The parameter decorator
  */
-declare function inject(token: InjectionToken<any>): (target: any, propertyKey: string | symbol, parameterIndex: number) => any;
+declare function inject(token: InjectionToken<any>): (target: any, propertyKey: string | symbol | undefined, parameterIndex: number) => any;
 export default inject;
diff --git a/node_modules/tsyringe/dist/typings/reflection-helpers.d.ts b/node_modules/tsyringe/dist/typings/reflection-helpers.d.ts
index 2591a35..4c45940 100644
--- a/node_modules/tsyringe/dist/typings/reflection-helpers.d.ts
+++ b/node_modules/tsyringe/dist/typings/reflection-helpers.d.ts
@@ -7,4 +7,4 @@ export declare function getParamInfo(target: constructor<any>): ParamInfo[];
 export declare function defineInjectionTokenMetadata(data: any, transform?: {
     transformToken: InjectionToken<Transform<any, any>>;
     args: any[];
-}): (target: any, propertyKey: string | symbol, parameterIndex: number) => any;
+}): (target: any, propertyKey: string | symbol | undefined, parameterIndex: number) => any;

@justinlevi
Copy link

I see that this PR was merged, do you know when the package.json version will be updated and a release will be published? Thanks

@zckrs
Copy link

zckrs commented May 24, 2023

Hi,

Someone can release a new tag (v4.8.0 maybe) ?

This unrelease change block many people to migrate with TS 5.

cc @MeltingMosaic @EliasBinde @EliTheSurfer

@johan13
Copy link

johan13 commented May 24, 2023

This unrelease change block many people to migrate with TS 5.

I agree that a new release would be good, but the workaround in #225 (comment) works fine in the meantime.

@jhechtf
Copy link

jhechtf commented May 27, 2023

It unfortunately does not work for me, as Deno is weirder with modules than Node. Please release the fix.

@tec27
Copy link

tec27 commented Jun 15, 2023

In case it's useful to anybody else, I have a fork of this repo with this fix included (and compiled files checked in, so it can be used as an npm dependency): https://github.com/tec27/tsyringe

You can use it by changing your package.json dep to:

"tsyringe": "tec27/tsyringe#43d00160db294312db4c7deb38046f71dcd74348"

tec27 added a commit to ShieldBattery/ShieldBattery that referenced this pull request Jun 15, 2023
Had to fork tsyringe to get the latest changes for the time being, this
can be undone once microsoft/tsyringe#226 is in
a published version.

Prettier and/or our import sorter also seemingly changed and wanted to
reorder capital letters so that is the majority of the changes here.
@JorgeLNJunior JorgeLNJunior deleted the fix/typescript5 branch June 28, 2023 17:58
@tobiasdiez
Copy link

Thanks @JorgeLNJunior for this fix. It works like a charm for me!

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

Successfully merging this pull request may close these issues.

TypeScript 5.0 Support of tighter parameter decorator checking