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

How to satisfy Server.adapter() with typeof Adapter? #3796

Closed
MickL opened this issue Feb 6, 2021 · 6 comments
Closed

How to satisfy Server.adapter() with typeof Adapter? #3796

MickL opened this issue Feb 6, 2021 · 6 comments
Labels
bug Something isn't working

Comments

@MickL
Copy link
Contributor

MickL commented Feb 6, 2021

Describe the bug
I am not sure if this is just a question or the typings are not correct, but it seems impossible for me to satisfy the Server.adapter() function which is defined as:

adapter(v: typeof Adapter): Server;

I tried:

createAdapter(): Adapter {}
createAdapter(): typeof Adapter {}
createAdapter(): ReturnType<Adapter> {}
createAdapter(): ReturnType<typeof Adapter> {}

Looking at socket.ioredis it just has no return type / returns any :/

@MickL MickL added the bug Something isn't working label Feb 6, 2021
@ironicbrew
Copy link

I think you may be importing Adapter from the wrong place, imported it improperly or it may be incorrectly declared wherever you are importing it from.

The exact type errors you are getting would be helpful here along with a more complete snippet/repo I could use to reproduce this error.

I figure this is the jist of what you are trying to accomplish except with your own custom Adapter class? The below throws no type issues on my end

import { Server } from "socket.io";
import {Adapter} from "socket.io-adapter";

function createAdapter(): typeof Adapter {
    return Adapter;
}

new Server(3333).adapter(createAdapter())

@darrachequesne
Copy link
Member

darrachequesne commented Feb 19, 2021

@MickL I think you are right, the typings are not correct.

The adapter argument should be a function that takes a namespace and returns an Adapter instance. Example:

@grubstarstar
Copy link

Isn't the problem that the types are using typeof Adapter instead of just using Adapter as it's already a type as it's a class.

e.g. If it change it to this:

adapter(): Adapter | undefined;
adapter(v: Adapter): this;
adapter(v?: Adapter): Adapter | undefined | this;

@darrachequesne
Copy link
Member

@grubstarstar the adapter function does not take a Adapter instance, but a function that takes a namespace and returns an Adapter instance (there is one Adapter instance per namespace, created here), so I don't think this would work.

I guess we should rather have something like type AdapterConstructor = typeof Adapter | ((nsp: Namespace) => Adapter);, to match the current implementation, but the Typescript compiler does not agree.

@MickL
Copy link
Contributor Author

MickL commented Mar 10, 2021

IMO the existing adapters should have a return type. Then they would run in the same problem as I did or solve my question.

Currently they dont have any return type thats why they throw no error:
https://github.com/socketio/socket.io-redis/blob/4dae265a740129b0d86f16418319b902a0648a3c/lib/index.ts#L70

Also the docs are wrong. The function does not return a RedisAdapter, instead it returns a function that may create a RedisAdapter:
https://github.com/socketio/socket.io-redis/blob/4dae265a740129b0d86f16418319b902a0648a3c/lib/index.ts#L66

@darrachequesne
Copy link
Member

This should be fixed by 891b187.

Please reopen if needed.

dzad pushed a commit to dzad/socket.io that referenced this issue May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants