uNetworking / uWebSockets.js

μWebSockets for Node.js back-ends :metal:
Apache License 2.0
7.86k stars 569 forks source link

Typesafe userdata in typescript #854

Closed gunhaxxor closed 1 year ago

gunhaxxor commented 1 year ago

Added possibility for the user to provide the shape of the user data with a type parameter when calling app.ws(). Like so:

type UserData = {
    info: string,
    token: string
}

uWebSockets.App().ws<UserData>(xxxx)

There has been a few attempts at this earlier that has been rejected for various reasons. Hopefully this PR checks all requirements. Let me know if/what is wrong or bad and I'll look into it 👍

Notes:

uNetworkingAB commented 1 year ago

export interface WebSocket { export type WebSocket = {

Why is WebSocket now a type? What's wrong with interface?

export interface HttpResponse { export interface HttpResponse<T = {}> {

This can't be correct. There is no T in HttpResponse.

uNetworkingAB commented 1 year ago

Also, for documentation T is a bad name please make it UserData like before. T is for brevity

gunhaxxor commented 1 year ago

As I've written above:

There was a wish from @uNetworkingAB to keep Websocket as an interface instead of a type alias. Unfortunately this isnt possible (I think), since interfaces don't support mapped types or can extend it's own generic type parameter. I guess the main reason for keeping Websocket an interface is to make it more clean in the generated documentation? If thats the case I personally believe this is worth the tradeoff.

Unfortunately I can't find a way to make the generic work while keeping the Websocket an interface. The problem boils down to interfaces in typescript being rather restrictive in how they can use/interact with the provided type parameter.

Regarding the generic on the httpResponse. There are to the best of my knowledge two options:

uNetworkingAB commented 1 year ago

Avoid making httpResponse a generic

Yep. Look at the C++ example there are two places you specify PerSocketData

https://github.com/uNetworking/uWebSockets/blob/4821d090fed3385ff1c3a76789b1de3fe2491ca7/examples/UpgradeSync.cpp#L36

uNetworkingAB commented 1 year ago

I made some basic changes and added getUserData() and now the following works nicely:

import * as uWS from "uWebSockets.js";

let app = uWS.App();

interface UserData {
    myMember: number;
}

app.ws<UserData>("/", {
    open: (ws) => {
        ws.getUserData().myMember = 13;
    }
});

This is the same interface as C++ so that's great

gunhaxxor commented 1 year ago

This is great! I think the API is even cleaner with the getUserData() as the userdata is then not intermingled with the properties on the websocket instance 👍

I guess the upgrade examples should be updated? I could probably submit a PR for that when I get some time over, but right now I'm quite overloaded with work 😐