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

An index signature parameter type must be either 'string', 'symbol' or 'number' #292

Open
rsteinig opened this issue Sep 7, 2022 · 10 comments
Labels
Bug Typescript Related to typescript definitions and integration

Comments

@rsteinig
Copy link

rsteinig commented Sep 7, 2022

Looks like, there is an issue by the latest 6.4.8 version. An index type can't be of any Uniontype definition. It must be broken down to any simple data type definition. It should be "[key in event]" instead. I use ng12.2.16 and TS 4.3.5.

export type event = (symbol|string);
export type eventNS = string|event[];
export type typeSafeEvents = {
[key: event]: (...args: any[]) => void
}

and

export declare class EventEmitter2<TypeSafeEvents extends typeSafeEvents = { [key: event]: (...args: any[]) => void }>

✖ Compiling with Angular sources in Ivy partial compilation mode.
node_modules/eventemitter2/eventemitter2.d.ts:4:6 - error TS1023: An index signature parameter type must be either 'string' or 'number'.

4 [key: event]: (...args: any[]) => void
~~~
node_modules/eventemitter2/eventemitter2.d.ts:124:79 - error TS1023: An index signature parameter type must be either 'string' or 'number'.

124 export declare class EventEmitter2<TypeSafeEvents extends typeSafeEvents = { [key: event]: (...args: any[]) => void }> {
~~~

@RangerMauve
Copy link
Contributor

Would you be able to submit a PR to fix this? Might be useful to pin dependencies to 6.4.7 until then. 😅

It'd be useful if we could get a PR for TS-specific tests to avoid breaking things like that.

@RangerMauve RangerMauve added Bug Typescript Related to typescript definitions and integration labels Sep 7, 2022
@rsteinig
Copy link
Author

rsteinig commented Sep 8, 2022

Would you be able to submit a PR to fix this? Might be useful to pin dependencies to 6.4.7 until then. sweat_smile

It'd be useful if we could get a PR for TS-specific tests to avoid breaking things like that.

If you advise me, how to do so =) For me it looks like a straight JS repository (https://github.com/EventEmitter2/EventEmitter2/blob/master/lib/eventemitter2.js), but I have no clue, how you generated that eventemitter2.d.ts file. Maybe I am missing sth, but would like to contribute. Or do you work directly inside that file, since I saw a PR linked to it?

@RangerMauve
Copy link
Contributor

If you advise me, how to do so =) For me it looks like a straight JS repository

Indeed we just add changes to the d.ts file directly. 😁 If you could update that to suite the type definitions you'd expect, that would be perfect.

@rsteinig
Copy link
Author

Just spent some time regarding the PR. Please advise me, how you want to proceed. Either you define your TS version ^4.4.4 or you use mapped object types by "in" for legacy TS versions.
Further, I can't commit and create and branch for PR. Need access ;-)

@sanketnadkarni7
Copy link

Ref: #292
We are encountering the same issue in eventEmitter2.d.ts after the version upgrade peer dependency of moleculer.js viz. 6.4.8, line number 4,which has emerged after the molecular’s latest build we generated on our production servers, this issue has blocked us from generating new builds for our deployment, can someone help resolving this or give us the timeline for resolution?
For Moleculer:
Raise a new issue with error name:
An index signature parameter type must be either 'string' or 'number'.
4 [key: event]: (...args: any[]) => void
And refer it with the link :
We have encountered this issue after one of the peer dependency in moleculer has some type issue i.e eventEmitter2 in version 6.4.8 (ttps://github.com//issues/292),
What can be done here?. Knowing that this is a peer dependency issue?. Can we expect a resolution from maintainers of moleculer eventEmitter?, we are having difficulties in deployment.

@RangerMauve
Copy link
Contributor

I'll revert the change and publish a new patch version to unblock folks while we figure out the best path forward. One sec.

@RangerMauve
Copy link
Contributor

6.4.9 has reverted the TS changes. Does that unblock your pipeline @sanketnadkarni7 ?

@RangerMauve
Copy link
Contributor

@rsteinig I think having something that supports legacy versions of TS would be best so that we don't cause too much pain to folks.

Would you mind creating a fork, making changes there, then doing a PR from your fork to this repo? I'll take note to release TS changes as major updates going forward just in case. 😅

@sanketnadkarni7
Copy link

@RangerMauve Yes it is working for us now, pipeline is passing now. Thankyou!

@rsteinig
Copy link
Author

@rsteinig I think having something that supports legacy versions of TS would be best so that we don't cause too much pain to folks.

Would you mind creating a fork, making changes there, then doing a PR from your fork to this repo? I'll take note to release TS changes as major updates going forward just in case. sweat_smile

just to let you know, PR got placed. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Typescript Related to typescript definitions and integration
Projects
None yet
Development

No branches or pull requests

3 participants