Skip to content

Conversation

@Marchand-Nicolas
Copy link
Collaborator

@Marchand-Nicolas Marchand-Nicolas commented Sep 3, 2025

Summary by CodeRabbit

  • New Requirements
    • Minimum Node.js version is now 22.
  • Chores
    • Upgraded dependencies to align with the latest ecosystem versions.
    • Updated type definitions and linting tools.
  • Refactor
    • Minor internal code cleanups and type improvements without changing behavior.
  • Tests
    • Updated test setup for environment variable loading.
  • Notes
    • No user-facing features changed; functionality remains the same.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 3, 2025

Walkthrough

Adds Node.js 22 settings (.nvmrc, engines in package.json), updates dependencies (eslint/Next/mongodb/types), converts a test to ESM dotenv import, and performs minor internal refactors (const usage, type tweaks, no-op callback style) without changing exported APIs or control flow.

Changes

Cohort / File(s) Summary
Runtime & tooling updates
package.json, .nvmrc
Adds .nvmrc with "22" and sets engines.node >=22. Bumps dependencies: eslint-config-next to ^14.2.0, mongodb to ^6.0.0, eslint to 8.57.0, and @types/* to Node 22/React 18 ranges. No code exports changed.
Type/const refactors (internal)
hooks/useHasClaimSolSubdomain.ts, utils/cacheDomainData.ts, utils/userDataService.ts
Switches some let to const; adjusts BigInt array type to bigint[]; removes an explicit Uint8Array annotation in favor of inference. No runtime behavior or API signatures changed.
Callback style (no-op)
hooks/useNotificationManager.ts
Replaces empty arrow function with block-bodied no-op containing a comment in a success callback. No logic change.
Test ESM dotenv import
tests/utils/apiWrappers/identity.test.ts
Replaces CommonJS require('dotenv') with ESM import dotenv from 'dotenv' and calls dotenv.config({ path: '.env.test' }). Test behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/update-to-node-22

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@vercel
Copy link

vercel bot commented Sep 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
app-starknet-id Ready Ready Preview Comment Sep 3, 2025 10:00am
sepolia-app-starknet-id Ready Ready Preview Comment Sep 3, 2025 10:00am

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
hooks/useNotificationManager.ts (2)

28-29: Guard against undefined address before hexToDecimal to prevent runtime errors.

hexToDecimal(undefined) can throw while wallet not connected.

Apply:

-      if (notification.address !== hexToDecimal(address)) return;
+      if (!address) return;
+      if (notification.address !== hexToDecimal(address)) return;
@@
-  const addTransaction = (notification: SIDNotification<NotificationData>) => {
-    setNotifications((prev) => [
-      { ...notification, address: hexToDecimal(address) },
-      ...prev,
-    ]);
-  };
+  const addTransaction = (notification: SIDNotification<NotificationData>) => {
+    if (!address) return;
+    setNotifications((prev) => [
+      { ...notification, address: hexToDecimal(address) },
+      ...prev,
+    ]);
+  };

Also applies to: 80-85


22-35: Avoid stale closures and unhandled rejections in polling.

forEach doesn’t await async callbacks; state writes use a captured snapshot. Wrap in try/catch and use functional state updates.

Apply:

-    const checkTransactionStatus = async (
+    const checkTransactionStatus = async (
       notification: SIDNotification<NotificationData>,
       index: number
     ) => {
+      try {
         if (notification.type !== NotificationType.TRANSACTION) return;
-      if (notification.address !== hexToDecimal(address)) return;
+        if (!address || notification.address !== hexToDecimal(address)) return;
         if (notification.data.status === "pending") {
           const transaction = notification.data;
           const transactionReceipt = await provider.waitForTransaction(
             transaction.hash
           );
-        const updatedTransactions = [...notifications];
+          // Use functional update to avoid stale state
+          setNotifications((prev) => {
+            const updated = [...prev];
+            if (!updated[index]) return prev; // index drift
@@
-          updatedTransactions[index].data.status = "error";
+            updated[index].data.status = "error";
           transactionReceipt.match({
             rejected: (txR: RejectedTransactionReceiptResponse) => {
-              updatedTransactions[index].data.txStatus = txR.status;
+              updated[index].data.txStatus = txR.status;
             },
             reverted: (txR: RevertedTransactionReceiptResponse) => {
-              updatedTransactions[index].data.txStatus = txR.status;
+              updated[index].data.txStatus = txR.status;
             },
             error: (err: Error) => {
               console.log("Error while fetching transaction receipt", err);
-              updatedTransactions[index].data.txStatus = undefined;
+              updated[index].data.txStatus = undefined;
             },
             success: () => {
               // Success callback - intentionally empty
             },
           });
-          setNotifications(updatedTransactions);
+            return updated;
+          });
         } else if (transactionReceipt.isSuccess()) {
-          updatedTransactions[index].data.txStatus =
-            transactionReceipt.finality_status;
-          updatedTransactions[index].data.status = "success";
-          setNotifications(updatedTransactions);
+          setNotifications((prev) => {
+            const updated = [...prev];
+            if (!updated[index]) return prev;
+            updated[index].data.txStatus = transactionReceipt.finality_status;
+            updated[index].data.status = "success";
+            return updated;
+          });
         }
-      }
+      } catch (e) {
+        console.warn("Polling tx status failed:", e);
+      }
     };
@@
-    const intervalId = setInterval(() => {
-      notifications.forEach(checkTransactionStatus);
-    }, 5000);
+    const intervalId = setInterval(() => {
+      // Fire-and-forget but bounded by try/catch inside
+      notifications.forEach((n, i) => { void checkTransactionStatus(n, i); });
+    }, 5000);

Also applies to: 41-63, 67-72

🧹 Nitpick comments (7)
.nvmrc (1)

1-1: Consider pinning a minor for determinism (optional).

"22" tracks the latest 22.x installed locally; if you want fully reproducible local envs, consider "22.x" in CI and documenting a baseline minor (e.g., 22.5+).

hooks/useHasClaimSolSubdomain.ts (1)

26-31: Use forEach/filter instead of map for side-effects; precompute decimal address.

Minor readability and micro-alloc reduction; also short-circuit on empty inputs.

Apply:

-    const domains: string[] = [];
-    (claimedData as bigint[]).map((claimAddr, index) => {
-      if (claimAddr.toString() === hexToDecimal(address)) {
-        domains.push(snsDomains[index]);
-      }
-    });
-    setClaimedDomains(domains);
+    if (!snsDomains?.length) {
+      setClaimedDomains([]);
+      return;
+    }
+    const dec = hexToDecimal(address);
+    const matched = (claimedData as bigint[]).reduce<string[]>((acc, claimAddr, i) => {
+      if (claimAddr.toString() === dec) acc.push(snsDomains[i]);
+      return acc;
+    }, []);
+    setClaimedDomains(matched);
hooks/useNotificationManager.ts (1)

53-55: No-op success callback is redundant in the error branch.

Optional: remove it to reduce noise.

tests/utils/apiWrappers/identity.test.ts (1)

2-4: No change needed for dotenv import. tsconfig.json has "esModuleInterop": true, so import dotenv from "dotenv" works correctly under your current setup. Optionally, you can centralize the dotenv.config({ path: ".env.test" }) call by adding it to Jest’s setupFiles in jest.config.js for DRY environment setup.

utils/cacheDomainData.ts (1)

14-15: Type the arrays to avoid implicit any[]

Declare explicit types to keep IdentityData shape enforced and help IntelliSense.

-  const extended_verifier_data = [];
-  const verifier_data = [];
+  const extended_verifier_data: IdentityData["extended_verifier_data"] = [];
+  const verifier_data: IdentityData["verifier_data"] = []
package.json (2)

51-53: Engines set to Node ≥22—ensure enforcement in CI/deploy

Add enforcement so dev/prod don’t drift.

  • CI: run on Node 22 in your matrix.
  • npm: add .npmrc with engine-strict=true (or enforce via your CI).
  • Vercel/hosting: confirm the project runtime is Node 22 (engines is usually honored, but verify).
    Do you want a PR to add CI updates?

35-35: MongoDB v6 usage verified
connectToDatabase uses the promise‐based MongoClient.connect() with no legacy callbacks or deprecated options—no breaking changes detected. Optionally, pass a ServerApiVersion in the MongoClient constructor for explicit API stability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7e3c291 and 7736587.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • .nvmrc (1 hunks)
  • hooks/useHasClaimSolSubdomain.ts (1 hunks)
  • hooks/useNotificationManager.ts (1 hunks)
  • package.json (2 hunks)
  • tests/utils/apiWrappers/identity.test.ts (1 hunks)
  • utils/cacheDomainData.ts (3 hunks)
  • utils/userDataService.ts (1 hunks)
🔇 Additional comments (6)
.nvmrc (1)

1-1: Verify engines.node and CI Node versions
I didn’t find an engines.node entry in package.json or any setup-node/node-version settings in your GitHub Actions workflows—please confirm both are explicitly set to Node 22 to match .nvmrc.

hooks/useHasClaimSolSubdomain.ts (1)

15-16: No action needed: HexString is globally declared in types/frontTypes.d.ts
The HexString type is defined as an ambient declaration in types/frontTypes.d.ts (included via tsconfig’s **/*.ts), so it’s already in scope—no import required.

utils/userDataService.ts (1)

24-24: LGTM: rely on type inference for encoder output.

Keeps the code idiomatic; no behavior change.

package.json (3)

31-31: ESLint Next config bump looks aligned with Next 14

No issues spotted; keep in sync with your Next version.


68-68: ESLint 8.57 OK

Looks compatible with your stack.


61-63: No action needed for @types/node ^22 upgrade – tsconfig.json already has "skipLibCheck": true to suppress DOM/Node fetch type conflicts, and Jest’s testEnvironment: "node" in jest.config.js (Node 18+ provides global.fetch) covers runtime.

Comment on lines +54 to 56
const existingDataRaw = localStorage.getItem("SID-domainData");
const existingData: Record<string, IdentityData> = existingDataRaw
? JSON.parse(existingDataRaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden JSON parse against corrupted localStorage

JSON.parse can throw; fall back safely to an empty map.

-  const existingDataRaw = localStorage.getItem("SID-domainData");
-  const existingData: Record<string, IdentityData> = existingDataRaw
-    ? JSON.parse(existingDataRaw)
-    : {};
+  const existingDataRaw = localStorage.getItem("SID-domainData");
+  let existingData: Record<string, IdentityData> = {};
+  if (existingDataRaw) {
+    try {
+      existingData = JSON.parse(existingDataRaw);
+    } catch {
+      existingData = {};
+    }
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const existingDataRaw = localStorage.getItem("SID-domainData");
const existingData: Record<string, IdentityData> = existingDataRaw
? JSON.parse(existingDataRaw)
const existingDataRaw = localStorage.getItem("SID-domainData");
let existingData: Record<string, IdentityData> = {};
if (existingDataRaw) {
try {
existingData = JSON.parse(existingDataRaw);
} catch {
existingData = {};
}
}
🤖 Prompt for AI Agents
In utils/cacheDomainData.ts around lines 54 to 56, the code calls JSON.parse on
localStorage data which can throw for corrupted or invalid JSON; wrap the parse
in a try/catch and on any error (or if parsed value is falsy/not an object) fall
back to an empty object {} and write that safe value back to localStorage or
keep using the empty map; ensure the variable existingData is correctly typed as
Record<string, IdentityData> and avoid letting JSON.parse exceptions bubble up.

Comment on lines +65 to 67
const existingDataRaw = localStorage.getItem("SID-domainData");
const existingData: Record<string, IdentityData> = existingDataRaw
? JSON.parse(existingDataRaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

SSR guard + safe parse in getDomainData

Next.js may import this on the server where localStorage is undefined; also protect JSON.parse.

Please confirm these utils are only called client-side today.

-export const getDomainData = (tokenId: string): IdentityData | undefined => {
-  const existingDataRaw = localStorage.getItem("SID-domainData");
-  const existingData: Record<string, IdentityData> = existingDataRaw
-    ? JSON.parse(existingDataRaw)
-    : {};
+export const getDomainData = (tokenId: string): IdentityData | undefined => {
+  if (typeof window === "undefined") return undefined;
+  const existingDataRaw = localStorage.getItem("SID-domainData");
+  let existingData: Record<string, IdentityData> = {};
+  if (existingDataRaw) {
+    try {
+      existingData = JSON.parse(existingDataRaw);
+    } catch {
+      return undefined;
+    }
+  }

🏁 Script executed:

#!/bin/bash
# Find all references to getDomainData in TypeScript/TSX files
rg -nP --type ts --type tsx "\\bgetDomainData\\b" -C3

# Find all references to localStorage in TypeScript/TSX files
rg -nP --type ts --type tsx "localStorage" -C3

Length of output: 156


🏁 Script executed:

#!/bin/bash
# Search for all usages of getDomainData across the repo
rg -nP '\bgetDomainData\b' -C3 .

# Search for all direct references to localStorage
rg -nP '\blocalStorage\b' -C3 .

Length of output: 11589


🏁 Script executed:

#!/bin/bash
# Check if availableIdentities.tsx is a client component via 'use client' directive
grep -n 'use client' components/identities/availableIdentities.tsx

# Preview the top of the file to inspect directives and imports
head -n 20 components/identities/availableIdentities.tsx

Length of output: 847


🏁 Script executed:

#!/bin/bash
# Search for availableIdentities import in pages directory
rg -n "availableIdentities" -g "pages/**/*.tsx"
# Search for getDomainData usage in pages directory
rg -n "getDomainData" -g "pages/**/*.tsx"

Length of output: 304


🏁 Script executed:

#!/bin/bash
# Print context around getDomainData invocation in the component
sed -n '110,150p' components/identities/availableIdentities.tsx

# Check if pages/identities.tsx is a client component and preview its top lines
grep -n 'use client' pages/identities.tsx || echo 'no client directive'
head -n 20 pages/identities.tsx

Length of output: 2381


Safe parse JSON in getDomainData
getDomainData is only invoked client-side, so no SSR guard is needed. Wrap the JSON.parse in a try/catch to prevent malformed data from throwing at runtime:

--- a/utils/cacheDomainData.ts
+++ b/utils/cacheDomainData.ts
@@ export const getDomainData = (tokenId: string): IdentityData | undefined => {
-  const existingDataRaw = localStorage.getItem("SID-domainData");
-  const existingData: Record<string, IdentityData> = existingDataRaw
-    ? JSON.parse(existingDataRaw)
-    : {};
+  const existingDataRaw = localStorage.getItem("SID-domainData");
+  let existingData: Record<string, IdentityData> = {};
+  if (existingDataRaw) {
+    try {
+      existingData = JSON.parse(existingDataRaw);
+    } catch {
+      console.warn("Invalid SID-domainData in localStorage");
+      return;
+    }
+  }
   return existingData[tokenId];
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const existingDataRaw = localStorage.getItem("SID-domainData");
const existingData: Record<string, IdentityData> = existingDataRaw
? JSON.parse(existingDataRaw)
export const getDomainData = (tokenId: string): IdentityData | undefined => {
const existingDataRaw = localStorage.getItem("SID-domainData");
let existingData: Record<string, IdentityData> = {};
if (existingDataRaw) {
try {
existingData = JSON.parse(existingDataRaw);
} catch {
console.warn("Invalid SID-domainData in localStorage");
return;
}
}
return existingData[tokenId];
};
🤖 Prompt for AI Agents
In utils/cacheDomainData.ts around lines 65 to 67, JSON.parse is called on
localStorage data without error handling which can throw on malformed data; wrap
the parse in a try/catch, fall back to an empty object (or null) on error, and
optionally remove the corrupted localStorage key to avoid repeated failures;
ensure the variable existingData is always assigned a valid Record<string,
IdentityData> before being used.

@fricoben fricoben merged commit c1147ee into testnet Sep 3, 2025
5 checks passed
fricoben added a commit that referenced this pull request Sep 3, 2025
* Fix: click on my identities to go back

* Fix: Build Error

* feat: fix generated address block styling

* fix: use accurate extrabold font

* fix: properly space elements as in figma

* fix: properly align images with elements in desktop

* fix: navbar aligns with figma

* feat: fix container shadow drop direction

* fix expiry date color

* Fix: Update wallet name from Argent to Ready in connection menu

* rename

* fix: restored navbar changes and updated the availabile idenitfies to fix the current issue

* fix: routing

* feat: update to node 22 (#1188)

---------

Co-authored-by: Verifieddanny <[email protected]>
Co-authored-by: Emmanuel Aroso <[email protected]>
Co-authored-by: Patrick-Ehimen <[email protected]>
Co-authored-by: Nico <[email protected]>
Co-authored-by: Nico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants