Skip to content
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

Improve web worker interface #1045

Open
wants to merge 76 commits into
base: main
Choose a base branch
from

Conversation

ymekuria
Copy link
Contributor

@ymekuria ymekuria commented Sep 19, 2024

Summary

This PR improves the implementation of integrating web workers in a UI with o1js. The comlink package was introduced to simplify the communication with web workers by abstracting message passing, and making them appear like regular asynchronous functions without the complexity of managing postMessage.

Impact

Currently web workers are necessary to build performant and usable UIs with o1js. Integrating and managing web workers with o1js is complex, and hard to manage according to many developers on discord. These developers have requested that we simplify the process. Integrating comlink simplifies the DX and allows more developers to build performant UIs with less complexity.

Changes

  • Introduced the comlink package
  • zkappWorker.ts and zkappWorkerClient.ts were simplified
  • Error handling was added in the example to aid debugging
  • The UI example was refactored so devs can more easily reason about it
  • The project was migrated from the NextJS page router to the app router
  • The dependancies were updated

Future Plans

The long term goal is to eliminate the need for web workers with updates to o1js. In the short term we are aiming to improve the DX of building UIs with changes like this one. This example implementation will be incrementally improved until we reach the longer term goal with improvements to o1js.

…ation with web workers for improved performance and concurrency
…mmunication to clean up the file and improve readability
…pi for better code organization and readability
…pdateTransaction method to use remoteApi with web worker
…ove functionality and state management in Home component
Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 1, 2024 6:32pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
07-oracles ⬜️ Ignored (Inspect) Visit Preview Oct 1, 2024 6:32pm

… functions by directly passing PublicKey object instead of an object with publicKey58 property
…blicKey directly without converting it to base58, as it is already in the correct format
… set up initial state, handle account existence checks, create and send transactions, refresh current state, and create UI elements for the Home page.
Copy link
Member

@Trivo25 Trivo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this is a great UX improvement! I think we should amend the actual tutorial too to include a better explanation of what Comlink does and how we use it

type Transaction = Awaited<ReturnType<typeof Mina.transaction>>;

const state = {
Add: null as null | typeof Add,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe calling this AddContract and renaming zkapp to contractInstance makes this a bit clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

@ymekuria
Copy link
Contributor Author

Cool, this is a great UX improvement! I think we should amend the actual tutorial too to include a better explanation of what Comlink does and how we use it

I will update the actual tutorial in a follow up PR.

… 'o1js' library and return API object to ensure worker api is availble after import
Copy link
Member

@querolita querolita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I am missing context about what it was like before. I can approve, but probably someone else should give you their opinion.

…ion link to clean up code and improve readability
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