tncrazvan / sveltekit-sse

Server Sent Events with SvelteKit
https://www.npmjs.com/package/sveltekit-sse
MIT License
204 stars 7 forks source link

Error using multiple events with .json() #25

Closed bluepuma77 closed 3 months ago

bluepuma77 commented 3 months ago

I have been trying all day to get this to work together, but it seems to not work.

It's the Chat component (link) of my demo app.

When using emit('users', JSON.stringify(getRoomUsers(room))) and

    const users = connection.select("users").json(
        function onJsonParseError({error, currentRawValue, previousParsedValue}){
            console.error(`Could not parse "${currentRawValue}" as json.`, error)
            return previousParsedValue  // this will be the new value of the store
        }
    )

I receive error Could not parse users as json

bluepuma77 commented 3 months ago

I created a Stackblitz REPL, the most basic version seems to work (link), need to dig deeper.

bluepuma77 commented 3 months ago

Now I added the new locking and pubsub to the basic example and it's not working anymore (link). This time I don't see any JSON parse error, but no SSE events are shown.

bluepuma77 commented 3 months ago

The reduced example works with a single emit(data) and not emit(eventName, data) (link).

Then I can receive without select():

const connection = source('/events')
  const value1 = connection.json(...)
tncrazvan commented 3 months ago

Hi @bluepuma77 , I'll take a look at this soon.

tncrazvan commented 3 months ago

Now I added the new locking and pubsub to the basic example and it's not working anymore (link). This time I don't see any JSON parse error, but no SSE events are shown.

@bluepuma77 you're using event() instead of events() to emit your messages.

The event function is meant for creating a single event over a single connection, more fit for http2 connections.

It gives you an EmitterOfOneEvent image

Which takes just 1 string as a parameter image

What you need is events() which gives you an EmitterOfManyEvents. image image

tncrazvan commented 3 months ago

@bluepuma77 here's the stackblitz example adjusted to use events instead of event (link).

bluepuma77 commented 3 months ago

Thanks for this, @tncrazvan ! Is it possible to log or throw an error when someone uses emit(name, data) with (single) event? I am sure there will be others that will make the same mistake.

tncrazvan commented 3 months ago

@bluepuma77 no, that's not an appropriate solution.

You need some linter for your project. If you do that, you'll get proper warnings about types, parameters and so on.

I just setup ESLint on the fork I made of your chatroom example, and this is the result

Peek 2024-01-27 19-52

As you can see, you get an actual linting error saying event() expects only 1 argument. You can even make it so that svelte doesn't build at all if there are errors, that is up to you though.

Obviously you don't have to use my ESLint configuration, there's even one that comes with any default sveltekit project if pick the option during creation

image

I just sent you another PR setting up ESLint for you on that project (link).

There's not much else I can do though, setting a logger for the emit() function might actually have a performance hit. I'm aware that speaking of "performance" in JavaScript is laughable, but still, it doesn't seem appropriate when there are other ways (with zero runtime cost) to prevent such mistakes.

I'm keeping this issue closed for now.