Closed cobward closed 2 years ago
Thanks, this is looking great. A few nits:
prettier
and set a style?If each branch is supposed to represent a version of the codebase with graduating maturity towards a full implementation, I think we should have all of them merged into main
but separated into different folders, such as:
00_initial
01_siwe_session
02_fetch_ens
03_fetch_nfts
Following up on Wayne's comment about styling, I used Airbnb's style guide for JS projects here (tzprofiles and rebase). It comes in both TS and JS flavors and uses ESLint to enforce style. It seems "closest" to idiomatic JS, but I'm open to other style guides if people prefer.
Here's a quick how-to: https://enlear.academy/how-to-set-up-airbnb-style-guide-82413ea6c5f2
Other than that, the project works fine locally and the code is very readable, good work!
- Could we end all JavaScript statements with a semicolon? Let's add
prettier
and set a style?
I was using standardjs, which removed all of the semicolons :facepalm: I'll do what you and Kelsey suggested.
Could you confirm that the message + nonce are loaded upon first page load, and not fetched over an endpoint in another step?
This is not the case. Currently the message is requested on the backend for each request (aka button click). I've added a sequence diagram to the docs to show this: https://app.gitbook.com/o/YHxbgvbOJwARzKZAVAA0/s/hsp8Jza9wShYT1iNJ5iM/c/YWokuqzuw7FX17a4jvRa/sign-in-with-ethereum/quickstart-guide/code-overview
Apologies if I've misunderstood what you wanted from this, I assumed each message needed to be different as there are three different actions (signIn
, load
and save
).
What's the relationship to Create the initial non-SIWE app. #1 ?
Currently there isn't a strong relationship between the two. Currently that initial
branch contains the starting point for the guide, and this branch is more than the completed version of the guide. The initial branch is completely unstyled, whereas this is fully styled.
Originally my intention was as you suggest, to have branches or directories containing the code at different stages that could be inspected. However, I'm not sure there are realistic steps that make sense. initial
-> siwe
contains the majority of the changes, siwe
-> fetch_ens
would be about 5 lines of javascript, and fetch_ens
-> fetch_nfts
again would be about another 5 lines.
We could "defactor" this branch to create a new initial
branch which has the nice styling, however I'm not sure that's necessarily a good idea. By using the "styled" branch as the example of what the code should look like after completing the guide, we would need to either show the styling code for the SIWE HTML elements in the guide (which I think would distract from the SIWE implementation details), or omit them which could lead to confusion when the guide-user tries to compare their code to this. If we do want to go this route, I think the latter would be better, but with some refactoring done to make a clear distinction between the functional and styling code.
I'm having a meeting with Michael at 15:15 UTC, so I'll make sure he's aware of the current state of play.
5 line changes sound great for illustrative purposes! I think it’s good to show how easy it is to add more features after integrating SIWE.
This is the SIWE app that the quickstart guide will show how to create (minus the nice visuals). For comparison the base non-SIWE app is on this branch: https://github.com/spruceid/siwe-quickstart/tree/feat/initial_app