robtaussig / react-use-websocket

React Hook for WebSocket communication
MIT License
1.63k stars 135 forks source link

filter callback function doesn't show the JSON parsed message #205

Open lucafaggianelli opened 1 year ago

lucafaggianelli commented 1 year ago

hi, I'm using the filter callback to filter messages, though that callback has a MessageEvent as argument, so I need to JSON.parse the message before applying the filter:

const filter = (msg: MessageEvent) => {
    const { type } = JSON.parse(msg.data)

    return type === topic
  }

then later I use the lastJsonMessage hook property so I guess I parse the message twice... is there another solution to avoid a double parse?

ewilliams-zoot commented 7 months ago

@lucafaggianelli I too have a pattern that would work really well to use the filtering, but I also don't want to parse twice, so I have to resort to using a string search in the filter function (which is better than parsing, but still not ideal).

Maybe some new options in the options param of useWebSocket? Because I also think it would be beneficial to opt out of JSON parsing if you aren't using JSON. Right now, the hook just catches a parse error and returns an empty object for the lastJsonMessage property.

Maybe there is a way to use an option, useJson?: boolean = true; that when true, will parse earlier on and send it through both the filter function and lastJsonMessage result.

lucafaggianelli commented 7 months ago

Yeah indeed I would add an argument for those not using json

ewilliams-zoot commented 7 months ago

Here is where the message is being captured in attach-listener.ts (also would need to do the same in attach-shared-listeners.ts):

webSocketInstance.onmessage = (message: WebSocketEventMap['message']) => {
    heartbeatCb?.();
    optionsRef.current.onMessage && optionsRef.current.onMessage(message);
    if (typeof optionsRef.current.filter === 'function' && optionsRef.current.filter(message) !== true) {
      return;
    }
    // ...
    setLastMessage(message);
};

That setLastMessage call triggers the hook to re-run, where it then parses the data into JSON for the lastJsonMessage return value. I think it would be possible to modify this section of code using an option to allow JSON or not, and when it is JSON, passing it into a filter and setting the JSON object directly instead of relying on a hook re-run to parse the raw message data again.

This does sort of break the generally-good rule of calculating values that can be derived from other state, instead of maintaining two pieces of state, but that might be justified by optimization.