-
Notifications
You must be signed in to change notification settings - Fork 816
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
fix: update document flow fetch logic #1039
Conversation
Important Auto Review SkippedAuto reviews are limited to the following labels: coderabbit. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Dumb question and I get why you've done it, but instead of maintaining the This would yield the same kind of result but without us managing it as directly. Edit: I realise above isn't really a question |
It's done this way so we can use the results from the TPRC mutations to update the relevant data, which reduces loading times Unless I'm misunderstanding what you're suggesting The query is just to ensure we refresh every now and then. |
Apologies, I seem to have a hard time expressing myself this week. My point is rather than doing this state locally you can use utils.setData to update the queryClient's data for the This would avoid the busy work of maintaining state ourself while also keeping everything updated without refetching. |
Ah I see I'll have a look. Pretty annoying it seems like we have to leave router refresh in since we need to clear the router cache for documents page 😩 Tempted to use trpc to render the documents page for client side at a later time |
46c96e1
to
63f6cde
Compare
Applied the changes and the PR is now ready for review |
I assume we will need to do the same for templates as well 😬 |
I retested your changes and they work, anything else to do for this? |
Lets do templates in a separate PR |
Description
Fixes issues with mismatching state between document steps.
For example, editing a recipient and proceeding to the next step may not display the updated recipient. And going back will display the old recipient instead of the updated values.
This PR also improves mutation and query speeds by adding logic to bypass query invalidation.
I've added workarounds to allow us to bypass things such as batching and invalidating queries. But I think we should instead remove this and update all the mutations where a query is required for a more optimised system.
Example benchmarks
Using stg-app vs this preview there's an average 50% speed increase across mutations.
Set signer step:
Average old speed: ~1100ms
Average new speed: ~550ms
Set recipient step:
Average old speed: ~1200ms
Average new speed: ~600ms
Set fields step:
Average old speed: ~1200ms
Average new speed: ~600ms
Related Issue
This will resolve #470
Changes Made
Checklist