Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughA new example React application demonstrating ENSNode primary name resolution is added to the repository. The project includes configuration files for Vite and TypeScript, React components that integrate with the ENSNode SDK via the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new minimal Vite + React example app under examples/ensnode-react-example to demonstrate using @ensnode/ensnode-react (specifically EnsNodeProvider + usePrimaryName) against a configurable ENSNode URL.
Changes:
- Introduces a new
examples/ensnode-react-exampleworkspace package with Vite/TS config and minimal UI. - Demonstrates primary-name resolution via
usePrimaryName, defaulting tohttps://api.alpha.ensnode.iowithVITE_ENSNODE_URLoverride. - Updates
pnpm-lock.yamlto include the new example importer and its dependencies.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds lockfile entries for the new example workspace importer and its resolved deps. |
| examples/ensnode-react-example/package.json | Defines the example workspace package, scripts, and dependencies. |
| examples/ensnode-react-example/vite.config.ts | Minimal Vite config enabling React plugin. |
| examples/ensnode-react-example/tsconfig.json | TypeScript configuration for the example source. |
| examples/ensnode-react-example/index.html | HTML entrypoint with #root mount point. |
| examples/ensnode-react-example/src/main.tsx | React 19 createRoot bootstrap rendering <App />. |
| examples/ensnode-react-example/src/App.tsx | Sets up EnsNodeProvider with URL-configurable options and renders the demo view. |
| examples/ensnode-react-example/src/PrimaryNameView.tsx | Implements the address input + usePrimaryName query UI. |
| examples/ensnode-react-example/src/vite-env.d.ts | Adds Vite client type references. |
| examples/ensnode-react-example/README.md | Documents usage and environment configuration. |
| examples/ensnode-react-example/LICENSE | Adds MIT license for the example package. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <form | ||
| onSubmit={(event) => { | ||
| event.preventDefault(); | ||
| setAddress(input as Address); | ||
| }} |
| <input | ||
| type="text" | ||
| value={input} | ||
| onChange={(event) => setInput(event.target.value)} | ||
| placeholder="0x..." | ||
| style={{ width: "28rem" }} | ||
| /> |
| }, | ||
| "dependencies": { | ||
| "@ensnode/ensnode-react": "workspace:*", | ||
| "@ensnode/ensnode-sdk": "workspace:*", |
There was a problem hiding this comment.
@notrab Yes I imagine this app may not need ensnode-sdk.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/ensnode-react-example/src/PrimaryNameView.tsx`:
- Around line 26-40: Validate the input before calling setAddress: replace the
blind type assertion setAddress(input as Address) with a check using viem's
isAddress (available via `@ensnode/ensnode-sdk`) and only call setAddress when
isAddress(input) is true; if invalid, prevent submission and surface feedback to
the user (e.g., set an error state and render a message) so the PrimaryNameView
form's onSubmit handler and related state (input, setInput, setAddress) validate
addresses rather than forwarding arbitrary text to usePrimaryName.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b70c8436-76c7-4a36-9d60-753f45611930
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
examples/ensnode-react-example/LICENSEexamples/ensnode-react-example/README.mdexamples/ensnode-react-example/index.htmlexamples/ensnode-react-example/package.jsonexamples/ensnode-react-example/src/App.tsxexamples/ensnode-react-example/src/PrimaryNameView.tsxexamples/ensnode-react-example/src/main.tsxexamples/ensnode-react-example/src/vite-env.d.tsexamples/ensnode-react-example/tsconfig.jsonexamples/ensnode-react-example/vite.config.ts
| <form | ||
| onSubmit={(event) => { | ||
| event.preventDefault(); | ||
| setAddress(input as Address); | ||
| }} | ||
| > | ||
| <input | ||
| type="text" | ||
| value={input} | ||
| onChange={(event) => setInput(event.target.value)} | ||
| placeholder="0x..." | ||
| style={{ width: "28rem" }} | ||
| /> | ||
| <button type="submit">Resolve</button> | ||
| </form> |
There was a problem hiding this comment.
Validate the address before submitting.
setAddress(input as Address) uses a type assertion to bypass validation, so any arbitrary text the user types is forwarded to usePrimaryName. Since this is the canonical example app, demonstrating address validation (e.g. via viem's isAddress, which is already transitively available through @ensnode/ensnode-sdk) would be a better pattern for users to copy. At minimum, surface invalid input to the user instead of relying on the hook's downstream error.
🛠 Suggested fix using viem's isAddress
+import { isAddress } from "viem";
import type { Address, ChainId } from "enssdk";
import { useState } from "react";
import { usePrimaryName } from "@ensnode/ensnode-react";
@@
export function PrimaryNameView() {
const [address, setAddress] = useState<Address>(DEFAULT_ADDRESS);
const [input, setInput] = useState<string>(DEFAULT_ADDRESS);
+ const [inputError, setInputError] = useState<string | null>(null);
@@
<form
onSubmit={(event) => {
event.preventDefault();
- setAddress(input as Address);
+ if (!isAddress(input)) {
+ setInputError("Invalid Ethereum address");
+ return;
+ }
+ setInputError(null);
+ setAddress(input);
}}
>
@@
<button type="submit">Resolve</button>
</form>
+ {inputError && <p>{inputError}</p>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/ensnode-react-example/src/PrimaryNameView.tsx` around lines 26 - 40,
Validate the input before calling setAddress: replace the blind type assertion
setAddress(input as Address) with a check using viem's isAddress (available via
`@ensnode/ensnode-sdk`) and only call setAddress when isAddress(input) is true; if
invalid, prevent submission and surface feedback to the user (e.g., set an error
state and render a message) so the PrimaryNameView form's onSubmit handler and
related state (input, setInput, setAddress) validate addresses rather than
forwarding arbitrary text to usePrimaryName.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Thanks for getting started on this PR. Noted how this was only a draft. Shared some suggestions with feedback.
| @@ -0,0 +1,20 @@ | |||
| # ensnode-react Example | |||
There was a problem hiding this comment.
It's super logical to put the ensnode-react-example inside examples/ensnode-react-example/ within the monorepo.
The context we have right now though is an unusual one. This ensnode-react-example app is important for us to ship right now for the purpose of internally refining how ensnode-react works so that we can make a plan for how best to integrate it into enskit. But at this time I don't think we want to be promoting the ensnode-react package. Therefore we also don't want to be promoting any example apps for ensnode-react.
I don't feel good about putting this app inside the root examples directory at this time. Could you please share your advice on a temporary alternative? For example, I'm thinking we stick it under docs temporarily.
These are all temporary steps, but everything we're doing here right now is very important. We need to have a maximally clear narrative to promote for how to integrate with ENSNode and that answer is enskit not ensnode-react. We therefore should take some special actions here now.
We also need to promote enskit integrations now and not later after we figure out how we want to integrate ensnode-react into enskit.
| }, | ||
| "dependencies": { | ||
| "@ensnode/ensnode-react": "workspace:*", | ||
| "@ensnode/ensnode-sdk": "workspace:*", |
There was a problem hiding this comment.
@notrab Yes I imagine this app may not need ensnode-sdk.
| @@ -0,0 +1,28 @@ | |||
| { | |||
| "name": "@ensnode/ensnode-react-example", | |||
There was a problem hiding this comment.
@notrab Can you please take the lead to wire up Vercel so that this example app can automatically get all the Vercel stuff working?
|
|
||
| import { usePrimaryName } from "@ensnode/ensnode-react"; | ||
|
|
||
| const DEFAULT_ADDRESS: Address = "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045"; // vitalik.eth |
There was a problem hiding this comment.
All address values we use in this app should use NormalizedAddress as imported from enssdk. You also need to call a utility function such as asNormalizedAddress on this hardcoded address string.
| <form | ||
| onSubmit={(event) => { | ||
| event.preventDefault(); | ||
| setAddress(input as Address); |
There was a problem hiding this comment.
@notrab I need to stop seeing code like this. Ever. It completely goes against our culture as a team and company. I'm going completely crazy at how many times I have to repeat the related ideas. There's so many problems in this 1 line of code I'm not going to spend the time to explain them all over and over again.
| <code>ensnode-react</code> Example App | ||
| </h1> | ||
| <p> | ||
| Connected to <code>{ENSNODE_URL}</code> |
There was a problem hiding this comment.
For this example app to achieve its goals, we need it to implement more critical ideas:
- How is it guaranteeing that it was able to successfully connect to the provided ENSNode?
- If for some reason it was unable to connect to the provided ENSNode, how is this case being gracefully handled? All the meaningfully distinct edge cases should have explicit handling. I don't want to see just a single "error connecting". I want more detailed error recognition such that you can disambiguate an error connecting (network-level error) from an unsupported ENSNode error (application-level error) where the config returned by the connected ENSNode is either not fetched or deserialized successfully.
- How is this app managing the latest indexing status projection / snapshot asynchronously in the background? I want to see a UI component for this in the example app similar to what's implemented for this inside the ENSAdmin UI (the little info icon).
- How is this app navigating the vital question of ENS Namespace? There's 2 options for this:
- Option 1: The example app hardcodes its own ENS Namespace configuration (ex: mainnet) and then upon connecting to an ENSNode instance, verifies that its config matches the expected namespace. If it doesn't, the example app should refuse the connection.
- Option 2: The example app doesn't hardcode any ENS Namespace configuration. Instead, it waits to connect to an ENSNode instance and then makes use of the whatever namespace that ENSNode instance's config is using.
I believe that Option 1 is the direction that 99% of our customers will want to use as there are benefits to knowing the ENS namespace even before a successful connection to ENSNode is established. Therefore this example app should implement Option 1.
That means this example app should also read an ENS namespace as an environment variable that defaults to mainnet.
| import { usePrimaryName } from "@ensnode/ensnode-react"; | ||
|
|
||
| const DEFAULT_ADDRESS: Address = "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045"; // vitalik.eth | ||
| const MAINNET: ChainId = 1; |
There was a problem hiding this comment.
This form shouldn't hardcode the chainId this way and also make reference to ENSIP-19 (which is multichain).
We should reference what we've already built for this idea before in ENSAdmin.
| @@ -0,0 +1,20 @@ | |||
| # ensnode-react Example | |||
|
|
|||
| A minimal React app demonstrating how to use `@ensnode/ensnode-react` to resolve | |||
There was a problem hiding this comment.
Please see my other comments with key ideas that should be included here for the key goals of this example app:
- The bigger comment I shared several lines above in this file.
- The bigger comment I shared in
examples/ensnode-react-example/src/App.tsx
I want to see this example app being an absolute A+ flawless bulletproof example of how an example app should manage the connection details with an ENSNode instance. This includes negotiating the connection itself, and the related validation and graceful / specialized error handling for that. It also includes maintaining an ongoing indexing status projection.
Todo
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)