137 feat integrate ensip 16 v2 to manage domain page #152
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…are both fomatted to the same data structure
| const newAddress = addresses.find( | ||
| (address) => address.label === addressField.label | ||
| )?.address as string; |
There was a problem hiding this comment.
| const newAddress = addresses.find( | |
| (address) => address.label === addressField.label | |
| )?.address as string; | |
| const inputtedNewAddressForLabel = addresses.find( | |
| (address) => address.label === addressField.label | |
| ); | |
| const addressValue = inputtedNewAddressForLabel || "" |
There was a problem hiding this comment.
@eduramme this is not resolved
There was a problem hiding this comment.
@FrancoAguzzi Please be more clear on future request changes. I've changed the function name.
the name inputtedNewAddressForLabel makes no sense in this context. The function responsibility is to populate the addresses tab context with values from the domain data, not inputted addresses.
Now I've created that fallback value to fix this 🤝🏻
There was a problem hiding this comment.
@eduramme as done in the other PR and in other previous:
- We do not want type aliasing. Whenever a type is aliased it shall be nice to say why (either with an explainable doc around why you are even needing to use this typescript joker or with
|| "") - We do not want
?.spread into the code so, the suggestion above is a conditional for wether the address is found or not, to set its value based on this.
The code shared above is pretty and well written, IMO
Yes, I might have mixed contexts on the name convention, please update it to addressesFieldsBasedOnDomainExistingData or something like
And sorry, what is the fallback value mentioned? Didn't catch it in code
There was a problem hiding this comment.
- Could you clarify what you mean by type aliasing? I don't believe it's relevant to the code here.
- Using conditional chaining
?.is a good practice when a value might beundefinedornull. In this case, we're filling address fields with ENS data, and since we can't always predict the response format, using?.or|| ""helps handle missing data safely.
Fallback implemented here: c3d8c21
const formattedNewAddress = newAddress || "";
Renaming of the variables implemented here: 78b0f94
Let me know if the code is clearer now
| name="Margaret Bourke" | ||
| variant="marble" |
There was a problem hiding this comment.
Perfect, @eduramme
I commented the issue, thanks for creating it. :)
| square | ||
| name="Margaret Bourke" | ||
| variant="marble" | ||
| colors={["#44BCF0", "#7298F8", "#A099FF", "#FFFFFF"]} |
There was a problem hiding this comment.
Sweet :) Perfect approach, @eduramme
|
|
||
| const metadataUrl = await publicClient.readContract({ | ||
| address: resolverAdd!, | ||
| abi: [parseAbiItem("function metadata() returns (string)")], |
There was a problem hiding this comment.
I see this failed: https://vercel.com/shared-blockfuls-projects/nameful/GcnLbAARRnYTPPv2ioQtd2Yfo3CQ?filter=errors
Can you investigate why ? Hopefully it is easy to fix 🧑🏼💻
It's actually inside a try...catch block. The I don't think this is the ideal behavior, but since we're focusing on the happy path to save time, I decided it's best not to worry about scenarios where a domain has no resolver for now. What do you think? |
| const newAddress = addresses.find( | ||
| (address) => address.label === addressField.label | ||
| )?.address as string; |
There was a problem hiding this comment.
@eduramme this is not resolved
No description provided.