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 typing of ExtensionStore.createInstance static #6764

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Dec 15, 2022

Signed-off-by: Sebastian Malton sebastian@malton.name

Regression from #6690

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 added bug Something isn't working area/extension Something to related to the extension api labels Dec 15, 2022
@Nokel81 Nokel81 added this to the 6.3.0 milestone Dec 15, 2022
@Nokel81 Nokel81 requested a review from a team as a code owner December 15, 2022 15:46
@Nokel81 Nokel81 requested review from jansav and aleksfront and removed request for a team December 15, 2022 15:46
Comment on lines +30 to +35
private static readonly instances = new WeakMap<object, any>();

/**
* @deprecated This is a form of global shared state. Just call `new Store(...)`
*/
static createInstance<T extends ExtensionStore<object>, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
static createInstance<T, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just reverting changes made in #6690, or are they typing changes needed because of refactoring there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in that PR I removed the extends Singleton from Basestore so to keep the same API of ExtensionStore I added the static methods here (because JS doesn't have multiple inheritance).

The ones on Singleton have T extends Singleton which is why I initially added the T extends ExtensionStore<object>, just it wasn't actually needed.

@Nokel81 Nokel81 merged commit fbaa9de into master Dec 15, 2022
@Nokel81 Nokel81 deleted the fix-typing-ExtensionStore branch December 15, 2022 21:29
@Nokel81 Nokel81 mentioned this pull request Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extension Something to related to the extension api bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants