Skip to content

Fix the issue where useFind suspense does not update the cache on the server side and improved performance #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

welkinwong
Copy link
Contributor

  1. On the server, useEffect doesn't run, so cached find data isn't cleared. Even after database updates, stale data is returned on subsequent requests.
  2. useFindSuspense can't run on the client (non-reactive data makes it pointless), so the useEffect part was removed.
  3. lodash.remove is no longer in use and can be removed.
  4. According to 7d45b72, the package-lock.json file can be deleted.

This PR builds on #429. The first commit is from the old PR, and the second commit adds new changes.

@radekmie
Copy link
Collaborator

useFindSuspense can't run on the client (non-reactive data makes it pointless), so the useEffect part was removed.

What do you mean by that? Of course it can! I agree, in most cases you'd go for the reactive useFind instead, but it's perfectly fine to use useFindSuspense on both client and server in isomorphic components.

@welkinwong
Copy link
Contributor Author

@radekmie Under what circumstances would one need to specifically import the non-reactive useFindSuspense for client-side use? The current import { useFind } from 'meteor/react-meteor-data/suspense'; can run on both the client and server because the code includes environment detection—if it's the client, it calls the useFindClient code, and only if it's the server does it call useFindSuspense.

Or could you tell me your specific use case?

@radekmie
Copy link
Collaborator

Ah, sorry, I misunderstood you before. In that case yes, that's a good change 👍

In strict mode (development only), useEffect may run 1-2 times. Throwing a promise outside can cause premature cleanup of subscriptions and cachedSubscription before unmount. To avoid this, check the timeout to ensure cleanup only occurs after unmount.
… server side and improved performance

1. On the server, useEffect doesn't run, so cached find data isn't cleared. Even after database updates, stale data is returned on subsequent requests.
2. useFindSuspense can't run on the client (non-reactive data makes it pointless), so the useEffect part was removed.
3. lodash.remove is no longer in use and can be removed.
4. According to 7d45b72, the package-lock.json file can be deleted.

This PR depends on #429
@welkinwong
Copy link
Contributor Author

I've updated the PR to handle cases where the collection name is null, which was an oversight in my previous implementation.

@welkinwong
Copy link
Contributor Author

Added tests for suspense/useFind to cover the scenarios mentioned in the PR.

before
Snipaste_2025-04-10_12-36-15

after
Snipaste_2025-04-10_12-34-24

@nachocodoner
Copy link
Member

Thank you for your contribution. I am verifying the changes and tests, and as there are two PR in parallel on this work, I use the other PR I can act as a way to resolve conflicts and consolidate the changes for next beta version.

Closing in favor of #435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants