-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
docs: add docs for tx deps prefetching optimisation #3499
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CodSpeed Performance ReportMerging #3499 will degrade performances by 44.35%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Dhai, do you think this could have it's own section in the docs around front-end optimisations? And I think those diagrams could do with some more love, maybe generate them with a flow chart tool.
|
||
This can be mitigated by preparing the contract call _before_ the user presses 'Submit Transaction'. If the transaction is prepared beforehand, the SDK only has to send the transaction to the network and wait for it to be confirmed. This reflects the actual speed of the chain to the user. | ||
|
||
You can experience this yourself by trying out this [demo](https://fuel-wallet-prefetch-experiment-75ug.vercel.app/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm should this be brought inside the SDK? Is it more beneficial for this to live inside a snippet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This = demo or the hook?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this URL come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The demo, usually we don't have any external deps for the docs, the links is from an external projects on Dhai's github
|
||
We recommend creating a `usePrepareContractCall` hook: | ||
|
||
```tsx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this all be in a snippet for forwards compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be; but that would mean us having to create a new project for snippets since this is a React snippet. In the interest of speed and considering how important this info is for our ecosystem projects, I think it is fine if we create the snippets in a separate PR. Nick wants us to announce this asap on Twitter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as we love teaching devs how to fetch data in React, we still have two more weeks of code freeze. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, this is not about teaching people how to fetch data in React. It's about the overall idea of preparing contract calls in advance, leading to a faster transaction experience.
About the code freeze, this is not adding any new code. Can we not get this in?
apps/docs/src/public/txdep1.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eliminated is the wrong wording here, as we are instead optimistically preparing the transaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it makes sense in the context of a tranasction's lifetime for the end user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielbate Agreed, the request is optimistically fetched, not eliminated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is eliminated from the contract call's lifecycle, so I strongly think it is eliminated in that context.
Co-authored-by: Daniel Bate <[email protected]>
Co-authored-by: Daniel Bate <[email protected]>
Co-authored-by: Daniel Bate <[email protected]>
Co-authored-by: Daniel Bate <[email protected]>
Co-authored-by: Daniel Bate <[email protected]>
@danielbate in the interest of getting this out there and speed; can we proceed with these diagrams for now? I personally think they are ok, but yes we could do better. Also: will we be able to merge/release this now during the code freeze? |
Coverage Report:
Changed Files:Coverage values did not change👌. |
{ | ||
text: 'Optimising Contract Calls', | ||
link: '/guide/contracts/optimising-contract-calls', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems confusing to mix React tips with core contract
documentation.
A simple React recipe demoing the example hook may suffice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree that this is a React tip. Like I mentioned here: #3499 (comment)
@@ -0,0 +1,106 @@ | |||
# Optimizing Contract Calls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not optimizing contract calls.
# Optimizing Contract Calls | |
# Optimizing Contract Estimation in React Apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, the estimation itself isn't optimized - it's just done in advance which leads to faster/optimised contract calls.
|
||
The below flowchart shows this entire process: | ||
|
||
![Transaction Lifecycle in the SDK without prefetching](../../public/txdep1.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm hesitant about adding images to the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
If there is an immediate need to communicate this to the community, this can be done independently to the TS SDK documentation. Could do this via a Twitter thread with diagrams and stackblitz rather than breaking the code freeze and pushing something we aren't all happy with. |
Release notes
In this release, we:
Summary
This PR adds documentation explaining how pre-fetching contract call/transaction dependencies can speed up transactions for users, and a recommended implementation for devs.
Checklist