redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.11k stars 981 forks source link

[Bug?]: Realtime context typescript error, no redis keys created #11316

Closed ladderschool closed 1 week ago

ladderschool commented 3 weeks ago

What's not working?

Unable to write to redis db when using realtime setup. Using similar code for setting up graphql subscriptions as the demo. Longer post here on discord.

There seems to be an issue with how the context is parsed:

{ context }: { context: { pubSub: NewNotificationChannelType } }

It appears to overwrite the global context?

How do we reproduce the bug?

Clone this repository.

You'll need a redis instance and secret in your .env file.

SESSION_SECRET = ...
REDIS_URL="redis://"

DbAuth is installed so you just sign up as a test user and log in. The home page has a button that increments a value, each time the resolver will publish a notification successfully. This part of realtime subscriptions work. The notification shows up and works properly with pubSub.

However, if you go to the resolver points.ts you'll notice a typescript error. You'll also notice that context.currentUser doesn't exist. When you increment the points and the resolver creates the record, there are no keys in the redis db even though it's set up in the realtime.ts file.

What's your environment? (If it applies)

Redwood 8.0.0-rc.1279
Windows 10

Are you interested in working on this?

ladderschool commented 3 weeks ago

After discussing with DT, it appears I misunderstood that realtime saves it as key/values in the actual redis db. That is not true, see here https://redis.io/docs/latest/develop/interact/pubsub/

The typescript error still persists and messes up the context, though.

dthyresson commented 3 weeks ago

I spoke to @ladderschool on Discord and clarified that pubSub messages in Redis are not persisted in keys where he has looking for them to be.

see https://redis.io/docs/latest/develop/interact/pubsub/

Closing issue.

dthyresson commented 3 weeks ago

Reopened as we will need to look at the TS issue

ladderschool commented 3 weeks ago

The fix branch has a proposed solution that seems to work, but I don't think that's supposed to be done that way?

fix branch

dthyresson commented 1 week ago

to be done that way

I actually think that's ok.

In all the examples I made, the subscription wasn't park of a Prisma call where the context would be different so had not seen this before.

Since we don't know how the subscription context is used, seem to let people define it as needed is best.