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

suggestion: refactor examples to be more readable #163

Closed
4 tasks
danisharora099 opened this issue Dec 7, 2022 · 11 comments
Closed
4 tasks

suggestion: refactor examples to be more readable #163

danisharora099 opened this issue Dec 7, 2022 · 11 comments
Labels
documentation Improvements or additions to documentation

Comments

@danisharora099
Copy link
Contributor

danisharora099 commented Dec 7, 2022

Problem

Our intention in offering examples for various use-cases/protocols is to help an end user understand how to use that particular protocol and the API.
While we might not want to provide a complete reference for the protocol, we do want to provide enough information to allow the user to understand how to use the protocol and the API.

IMO, the current state of examples with the JS within the HTML and using the script tag is not ideal. I think we should move to a model where we have a separate JS file for each example and we use the script tag to load the JS file. We could also move away from HTML, into an easy to understand and read file structure that a framework like React offers (or any other framework).
This will allow us to use a linter to check the JS code, while helping stay consistent on git with the prettier config etc also.

Icing on the cake will be converting the JS code into TS, increasing the readability of the code further, while also helping us (core contributors) in refactoring/working with the code with type definitions in place.
It also helps that all libraries that these examples rely on, already have the type definitons.

I am not sure if this is the right place to discuss this, but I think this is a good place to start the discussion.

Thoughts?

Acceptance Criteria

  • Create a check list for all examples here
  • Review the code of each example, identify possible improvements in API and example code
  • Examples that are only index.html should have JS code extracted to index.js
  • Move all React examples to use @waku/react
@danisharora099
Copy link
Contributor Author

danisharora099 commented Dec 7, 2022

For context: I faced and realised the different friction points while working on the js-rln example

@weboko
Copy link
Contributor

weboko commented Dec 7, 2022

I think this is a good idea - to revamp existing examples but I wouldn't get rid entirely of HTML/JS examples because this is still a valuable source for reference. In my opinion those examples should be simple & robust. Splitting JS file from HTML for sure is a must have.
One of the reasons we should keep HTML/JS is to target developers that are not using TS/React/Angular and will try to find a reference for their problem without searching for those keywords.

@fryorcraken
Copy link
Collaborator

and read file structure that a framework like React offers (or any other framework).

We have a mix of framework on purpose: React. Angular, no framework, and open to add more examples for more frameworks such as Vue.

However, ... well I came here to say what @weboko already said: #163 (comment)

But I do agree that splitting Javascript to index.js file is a good idea.

Also, note that the examples are a good way to dogfood the API and spot possible improvements such as:

  • API difficult to use -> make API easier
  • Dev needs to write code that they shouldn't need too -> provide helper functions/get API to do it for them
  • etc

@fryorcraken
Copy link
Collaborator

fryorcraken commented Dec 13, 2022

Some comments re js-rln examples:

Currently the user has to click at each step. This is because RLN is a new protocol and we wanted to provide some transparency to a potential builder so they understand how the protocol works.

We may want to move away from that and closer to how it would like if someone were to integrate it while still providing clarity. For example., list all the steps and display a tick as the steps are done:

For example:

Download WASM file... ✔️
Retrieve data from contract... ✔️
Connect to remote Waku node...

One note though is as zerokit changes and is integrated in nwaku, we may reach stages were the wakuv2.prod fleet is not compatible with latest zerokit. Hence we enable the user to enter a multiaddr for a node to target a different fleet or even a local node.

Maybe this is not needed anymore and we just point to the test fleet for now.

@s1fr0 wdyt in terms of stability of RLN in nwaku and backward compatibility across versions?

@s1fr0
Copy link

s1fr0 commented Dec 13, 2022

The idea is to not break APIs whenever possible. However recently we refactored the circuit increasing security, so it is likely in the future that we break again APIs (eventually with a transition phase to progressively deprecate broken APIs).

At the moment I see at least one other upcoming breaking change to increase circuit efficiency (bring out of the circuit the hash computation for external_nullifier).

However, zerokit rln looks mature enough now to start periodical releases, hence I expect prod fleet to follow new releases only with all the necessary protocol bumps.
At the same time, we already release nightly builds with artifacts which are already downloaded (in place of compilation) when pushing new PRs and nwaku test fleet deployment should be triggered automatically at every new merge to master. Hence, test fleet will automatically get the latest zerokit rln version.

In this sense, a multiaddr make sense to me to allow the user to have stability on one side but also the possibility to try new experimental features.

@fryorcraken
Copy link
Collaborator

More details on rln-js.

Currently, the example has glue code that extract events from the smart contract using ethers, and then insert the memberships (extracted from events) into the rln instance. Finally, it also listens to new events.

We expect any dev to do this action to use RLN, the only option is whether they use ethers or web3js or another lib.

So it may makes sense to provide a function that handles it for you with ethers (and ethers could be an optional dependency for @waku/rln).

What could be added to @waku/rln:

  • contract details: ABI, address, block deployment for Goerli smart cotnract
  • handleMembership logic
  • retrieveRLNDetailsButton.onclick logic
  • registerButton.onclick logic

@danisharora099
Copy link
Contributor Author

@hackyguru this issue might be helpful for you

@fryorcraken fryorcraken added documentation Improvements or additions to documentation API labels Mar 23, 2023
@fryorcraken
Copy link
Collaborator

As mentioned here: #232 (comment)

We currently have a dilemma when dealing with each example:

  1. Make it super simple so it's easy to understand and replicate
  2. Make it complete enough so it can be a good PoC to demonstrate features.

Had a chat with @LordGhostX and one proposal that I think make sense is to move the simple examples to the docs. They live in there as guides. In the future, we could even add a sandbox to each guide where someone can play with the final result in the sandbox. Then, keep the PoCs in the present repo. Code still need to be clear and understandable but PoCs focus on demonstrating features.

We can talk this as @LordGhostX writes more guides of existing examples.

@weboko
Copy link
Contributor

weboko commented Oct 27, 2023

add a sandbox to each guide where someone can play with the final result in the sandbox

@fryorcraken, we shouldn't loose this idea, if we don't have an issue - let's make one
I have experience setting up something similar in https://codepen.io/ and https://jsfiddle.net/ but https://replit.com/ can also work. First step is fairly easy and can give huge help during hackathons
cc @hackyguru

for other parts of this issue I believe they will be addressed in #275 so it should be safe to close this one

@fryorcraken
Copy link
Collaborator

@weboko sounds good. I think le'ts open an issue on docs.waku.org to setup a sandbox.

@weboko
Copy link
Contributor

weboko commented Oct 30, 2023

Created issue waku-org/docs.waku.org#129

@weboko weboko closed this as completed Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

No branches or pull requests

4 participants