feat(enskit-react-example): add alpha instance selector - #2299
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughReplaces the static module-level ENSNode client ( ChangesENSNode Instance Selection Provider
Sequence Diagram(s)sequenceDiagram
participant User
participant InstanceSelector
participant EnsnodeInstanceProvider
participant localStorage
participant OmnigraphProvider
participant NamegraphView
User->>InstanceSelector: select new instance
InstanceSelector->>EnsnodeInstanceProvider: setInstanceId(id)
EnsnodeInstanceProvider->>localStorage: persist ENSNODE_INSTANCE_STORAGE_KEY
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: update instance query param (replace)
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: recompute ensnodeUrl, memoize new client
EnsnodeInstanceProvider->>OmnigraphProvider: remount keyed by ensnodeUrl
OmnigraphProvider->>NamegraphView: new client via useEnsnodeInstance()
NamegraphView->>NamegraphView: re-fetch registry page with new client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/enskit-react-example/src/EnsnodeInstanceProvider.tsx`:
- Around line 50-53: The EnsnodeInstanceProvider component initializes
instanceId from searchParams only once without synchronizing when the URL
changes. Add a useEffect hook that listens to searchParams as a dependency and
calls setInstanceIdState with the resolved value whenever searchParams changes.
This ensures that browser navigation (back/forward) or deep links with different
instance query parameters will properly update the instanceId state to match the
current URL.
🪄 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: 8da28fcb-5d05-4eec-9485-510856b6b341
📒 Files selected for processing (8)
examples/enskit-react-example/src/App.tsxexamples/enskit-react-example/src/DomainView.tsxexamples/enskit-react-example/src/EnsnodeInstanceProvider.tsxexamples/enskit-react-example/src/NamegraphView.tsxexamples/enskit-react-example/src/SearchView.tsxexamples/enskit-react-example/src/client.tsexamples/enskit-react-example/src/ensnode.tsexamples/enskit-react-example/src/instances.ts
💤 Files with no reviewable changes (1)
- examples/enskit-react-example/src/ensnode.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/enskit-react-example/src/instances.ts (2)
1-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd JSDoc to document the type alias invariants.
The coding guidelines require that "Type aliases must document invariants; each invariant MUST be documented exactly once on its type alias". This interface should have JSDoc explaining its purpose and any invariants (e.g.,
defaultAddressmust be a valid Ethereum address,defaultDomainNamemust be a valid ENS domain).📝 Example JSDoc documentation
+/** + * Per-instance configuration constants used for defaults throughout the UI. + * Invariants: + * - defaultAddress must be a valid checksummed Ethereum address + * - defaultDomainName must be a valid ENS domain name + * - defaultSearchLabel must be a non-empty string + */ export interface EnsnodeInstanceConstants { defaultAddress: string; defaultSearchLabel: string; defaultDomainName: string; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/enskit-react-example/src/instances.ts` around lines 1 - 5, Add JSDoc documentation to the EnsnodeInstanceConstants interface to document its purpose and invariants as required by the coding guidelines. The JSDoc should include a description of the interface's purpose and clearly document the invariants for each property (for example, that defaultAddress must be a valid Ethereum address, defaultDomainName must be a valid ENS domain, etc.). Place this JSDoc comment immediately above the interface declaration.Source: Coding guidelines
7-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd JSDoc to document the type alias invariants.
Per coding guidelines, type aliases must document their invariants. This interface should explain its purpose and constraints (e.g.,
idmust be unique across all instances,urlmust be a valid HTTPS endpoint).📝 Example JSDoc documentation
+/** + * Represents a single ENSNode instance configuration. + * Invariants: + * - id must be unique within ENSNODE_INSTANCES + * - url must be a valid HTTPS endpoint + * - label is displayed in the instance selector UI + * - constants provides per-instance default values + */ export interface EnsnodeInstance { id: string; url: string; label: string; constants: EnsnodeInstanceConstants; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/enskit-react-example/src/instances.ts` around lines 7 - 12, The EnsnodeInstance interface is missing JSDoc documentation that describes its purpose and invariants. Add a JSDoc comment block above the EnsnodeInstance interface definition that explains the interface's purpose and documents the constraints for each property, including that the id must be unique across all instances, the url must be a valid HTTPS endpoint, and any other relevant invariants for the label and constants properties.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/enskit-react-example/src/app-paths.ts`:
- Around line 21-25: The withInstanceSearch function on lines 21-25 always uses
a question mark as the separator when appending the instance parameter, but if
the input path already contains query parameters, this creates malformed URLs.
Fix this by checking whether the path already contains a question mark before
constructing the final URL. If the path already has a question mark, use an
ampersand as the separator; otherwise, use a question mark. This ensures proper
query parameter formatting regardless of whether the input path has existing
parameters.
In `@examples/enskit-react-example/src/instances.ts`:
- Around line 15-36: The hardcoded ENSNODE_INSTANCES array lacks validation,
allowing configuration errors to potentially slip into runtime. Create a Zod
schema that matches the EnsnodeInstance type structure (including validations
for URLs, Ethereum addresses, and required fields like id, url, label, and
constants), then use zod.parse() to validate the ENSNODE_INSTANCES array at
module load time. This ensures any malformed configuration data or invalid field
values are caught immediately when the module loads.
---
Outside diff comments:
In `@examples/enskit-react-example/src/instances.ts`:
- Around line 1-5: Add JSDoc documentation to the EnsnodeInstanceConstants
interface to document its purpose and invariants as required by the coding
guidelines. The JSDoc should include a description of the interface's purpose
and clearly document the invariants for each property (for example, that
defaultAddress must be a valid Ethereum address, defaultDomainName must be a
valid ENS domain, etc.). Place this JSDoc comment immediately above the
interface declaration.
- Around line 7-12: The EnsnodeInstance interface is missing JSDoc documentation
that describes its purpose and invariants. Add a JSDoc comment block above the
EnsnodeInstance interface definition that explains the interface's purpose and
documents the constraints for each property, including that the id must be
unique across all instances, the url must be a valid HTTPS endpoint, and any
other relevant invariants for the label and constants properties.
🪄 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: 712d9019-f755-4d39-ba6b-a08922835b15
📒 Files selected for processing (7)
examples/enskit-react-example/src/AccountView.tsxexamples/enskit-react-example/src/App.tsxexamples/enskit-react-example/src/DomainView.tsxexamples/enskit-react-example/src/NamegraphView.tsxexamples/enskit-react-example/src/SearchView.tsxexamples/enskit-react-example/src/app-paths.tsexamples/enskit-react-example/src/instances.ts
|
@greptile review |
Greptile SummaryThis PR adds a runtime ENSNode instance selector to the
Confidence Score: 4/5
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant InstanceSelector
participant EnsnodeInstanceProvider
participant localStorage
participant HashRouter (URL)
participant OmnigraphProvider
participant Views (Layout / DomainView / etc.)
User->>InstanceSelector: selects "V2 Sepolia"
InstanceSelector->>EnsnodeInstanceProvider: setInstanceId("v2-sepolia")
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: setInstanceIdState("v2-sepolia")
EnsnodeInstanceProvider->>localStorage: setItem(key, "v2-sepolia")
EnsnodeInstanceProvider->>HashRouter (URL): setSearchParams({instance: "v2-sepolia"}, replace)
HashRouter (URL)-->>EnsnodeInstanceProvider: searchParams changed → useEffect fires
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: resolveInstanceId → "v2-sepolia" (same, no re-render)
EnsnodeInstanceProvider->>OmnigraphProvider: "new client + key=newUrl forces remount"
OmnigraphProvider-->>Views (Layout / DomainView / etc.): fresh query context
Views (Layout / DomainView / etc.)->>Views (Layout / DomainView / etc.): useAppPath() preserves ?instance=v2-sepolia in all Links
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant InstanceSelector
participant EnsnodeInstanceProvider
participant localStorage
participant HashRouter (URL)
participant OmnigraphProvider
participant Views (Layout / DomainView / etc.)
User->>InstanceSelector: selects "V2 Sepolia"
InstanceSelector->>EnsnodeInstanceProvider: setInstanceId("v2-sepolia")
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: setInstanceIdState("v2-sepolia")
EnsnodeInstanceProvider->>localStorage: setItem(key, "v2-sepolia")
EnsnodeInstanceProvider->>HashRouter (URL): setSearchParams({instance: "v2-sepolia"}, replace)
HashRouter (URL)-->>EnsnodeInstanceProvider: searchParams changed → useEffect fires
EnsnodeInstanceProvider->>EnsnodeInstanceProvider: resolveInstanceId → "v2-sepolia" (same, no re-render)
EnsnodeInstanceProvider->>OmnigraphProvider: "new client + key=newUrl forces remount"
OmnigraphProvider-->>Views (Layout / DomainView / etc.): fresh query context
Views (Layout / DomainView / etc.)->>Views (Layout / DomainView / etc.): useAppPath() preserves ?instance=v2-sepolia in all Links
Reviews (1): Last reviewed commit: "fix cicd" | Re-trigger Greptile |
Greptile SummaryThis PR adds an alpha-instance selector to the
Confidence Score: 4/5Safe to merge; changes are confined to the example app and introduce no regressions to the core library. The refactor is well-structured and internally consistent. The two findings are both non-blocking quality suggestions: DEFAULT_ENSNODE_INSTANCE relies on array-index assignment without a guard, and useAppPath allocates a new function reference on every render. Neither causes incorrect behaviour today, but both could catch future contributors off-guard. instances.ts (DEFAULT_ENSNODE_INSTANCE guard) and app-paths.ts (useAppPath memoization) are worth a second look, though neither is blocking. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[App / HashRouter] --> B[EnsnodeInstanceProvider]
B --> C{resolveInstanceId}
C -->|URL ?instance=| D[URLSearchParams]
C -->|fallback| E[localStorage]
C -->|fallback| F[DEFAULT_ENSNODE_INSTANCE]
B --> G[EnsnodeInstanceContext.Provider]
G --> H["OmnigraphProvider\n(key=ensnodeUrl)"]
H --> I[Layout / Routes]
I --> J[InstanceSelector\n select dropdown]
I --> K[Views\nDomain / Account / Search / Namegraph]
J -->|setInstanceId| L[Update state + localStorage + URL]
L -->|searchParams change| B
K -->|useAppPath| M["withInstanceSearch\n(propagates ?instance= param)"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[App / HashRouter] --> B[EnsnodeInstanceProvider]
B --> C{resolveInstanceId}
C -->|URL ?instance=| D[URLSearchParams]
C -->|fallback| E[localStorage]
C -->|fallback| F[DEFAULT_ENSNODE_INSTANCE]
B --> G[EnsnodeInstanceContext.Provider]
G --> H["OmnigraphProvider\n(key=ensnodeUrl)"]
H --> I[Layout / Routes]
I --> J[InstanceSelector\n select dropdown]
I --> K[Views\nDomain / Account / Search / Namegraph]
J -->|setInstanceId| L[Update state + localStorage + URL]
L -->|searchParams change| B
K -->|useAppPath| M["withInstanceSearch\n(propagates ?instance= param)"]
Reviews (2): Last reviewed commit: "fix cicd" | Re-trigger Greptile |
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Thanks for updates. Please take the lead to merge when you're ready 👍
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
Why
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)