meteor / react-packages

Meteor packages for a great React developer experience
http://guide.meteor.com/react.html
Other
574 stars 159 forks source link

Stringify params for useSubscribe to avoid stopping the subscription when object reference changes #412

Closed denihs closed 2 months ago

denihs commented 2 months ago

This issue was reported in our Discord.

This is a video showing the issue:

https://github.com/user-attachments/assets/7490bcb1-dc34-4c39-a2a3-f84f14338156

If you check the part of the code where we call useSubscribe,, the params remain the same.

However, the cleanup of the useEffect is still triggered because we change the object reference after unmounting the page.

To solve this, we're now stringifying the params. This could cause an issue in different scenarios if the object is too big, but in this case, we are stringifying the parameters for a subscription, so it's unlikely this object will cause any issue.

Here is the result after this change:

https://github.com/user-attachments/assets/9130e02b-85b8-474b-9863-1a24bc2138bd

image

So everything now works as expected.

PS: My example is slightly different from the one done by the issue reporter. In my example, I changed the parameter in the URL to validate if the subscription with the older parameter would be dropped.

dburles commented 2 months ago

Is the order of EJSON.stringify deterministic? react-query for instance will sort objects in query keys (see: https://github.com/TanStack/query/blob/main/packages/query-core/src/utils.ts#L205-L216).

A more robust approach is to force users to use string keys. eg. useSubscribe("user-profile", `user-profile-${userId}`, { userId });

denihs commented 2 months ago

A more robust approach is to force users to use string keys

It would, but the usability would be bad.

Let's try it like this for now.