Conversation
📝 WalkthroughWalkthroughThe changes add a new development dependency for on-chain contract integration and refactor the documentation generation script to dynamically generate a Deployed Addresses page from the newly added contracts package, replacing the previous hard-coded approach. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/sync-contracts-docs.js`:
- Around line 155-159: The loop over Object.entries(addresses) currently drops
unknown chain IDs with if (!chain) continue; instead create a fallback chain
object so unknown chains are rendered rather than skipped: when CHAINS[chainId]
is falsy, set a fallback like { id: chainId, name: `chain-${chainId}` } (or a
descriptive label) and use that fallback for subsequent rendering logic (replace
usages of chain with this fallbackChain variable); ensure any downstream
properties accessed (e.g., chain.name or chain.id) are read from the fallback so
new deployments appear in the docs.
- Around line 23-28: The try/catch around
require('@yellow-org/contracts').addresses is too broad and hides real errors;
update the try/catch in scripts/sync-contracts-docs.js that assigns addresses so
it only swallows a MODULE_NOT_FOUND error and rethrows any other
exceptions—i.e., after calling require('@yellow-org/contracts') (the expression
used to set addresses), inspect the caught error.code and if it equals
'MODULE_NOT_FOUND' set addresses = null, otherwise throw the error so issues
like syntax or export problems surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5444538-8f8e-404a-8074-37db6004e5ef
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonscripts/sync-contracts-docs.js
| let addresses; | ||
| try { | ||
| addresses = require('@yellow-org/contracts').addresses; | ||
| } catch { | ||
| addresses = null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package*.json" -type f | head -20Repository: layer-3/docs
Length of output: 90
🏁 Script executed:
rg "@yellow-org/contracts" --type jsonRepository: layer-3/docs
Length of output: 340
🏁 Script executed:
git ls-files scripts/ | head -20Repository: layer-3/docs
Length of output: 165
🏁 Script executed:
cat -n scripts/sync-contracts-docs.js | head -50Repository: layer-3/docs
Length of output: 1947
🌐 Web query:
@yellow-org/contracts npm module exports addresses CommonJS
💡 Result:
If @yellow-org/contracts is published as ESM-only (common for “contracts artifacts” packages), then CommonJS require() won’t give you named exports the way you expect.
How to consume it from CommonJS
Option A (recommended): use dynamic import() from CJS
// CommonJS file (.cjs or "type": "commonjs")
const { addresses } = await import('@yellow-org/contracts');
// or whatever export name the package uses
console.log(addresses);Node can load ESM via import(), and when importing CommonJS from ESM (or vice‑versa), interop often goes through a default export wrapper. (mirrors.aliyun.com)
Option B: require() and look for .default
const pkg = require('@yellow-org/contracts');
const addresses = pkg.addresses ?? pkg.default?.addresses ?? pkg.default;This is needed when Node represents the other module system via a namespace object whose main value is on default. (mirrors.aliyun.com)
Option C: if you control the package, add conditional exports
In the package’s package.json, publish both entry points:
{
"exports": {
".": {
"import": "./dist/index.mjs",
"require": "./dist/index.cjs"
}
}
}This is the standard way to support both import and require. (nodejs.org)
If require() throws “ERR_REQUIRE_ESM”
That means it’s ESM-only; use Option A (dynamic import()), or ship a CJS build + conditional exports (Option C). Node only supports requiring ESM in limited/experimental ways. (mirrors.aliyun.com)
Citations:
- 1: https://mirrors.aliyun.com/nodejs-release/v22.7.0/docs/api/esm.html?utm_source=openai
- 2: https://mirrors.aliyun.com/nodejs-release/v22.7.0/docs/api/esm.html?utm_source=openai
- 3: https://nodejs.org/download/release/v17.4.0/docs/api/packages.html?utm_source=openai
- 4: https://mirrors.aliyun.com/nodejs-release/v22.7.0/docs/api/esm.html?utm_source=openai
🏁 Script executed:
cat -n package-lock.json | grep -A 20 "@yellow-org/contracts"Repository: layer-3/docs
Length of output: 2139
🏁 Script executed:
rg "@yellow-org/contracts" --type js -A 3 -B 3Repository: layer-3/docs
Length of output: 3341
🏁 Script executed:
cat -n scripts/sync-contracts-docs.js | tail -n +20 | head -n 50Repository: layer-3/docs
Length of output: 1805
Avoid broad catch blocks—only catch missing module errors.
This catches every runtime failure and silently degrades to addresses = null, which masks real module issues (broken exports, syntax errors, incompatible formats). Catch MODULE_NOT_FOUND specifically and rethrow unexpected errors so actual problems surface during development.
Proposed fix
let addresses;
try {
addresses = require('@yellow-org/contracts').addresses;
-} catch {
- addresses = null;
+} catch (err) {
+ if (err && err.code === 'MODULE_NOT_FOUND') {
+ addresses = null;
+ } else {
+ throw err;
+ }
}📝 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.
| let addresses; | |
| try { | |
| addresses = require('@yellow-org/contracts').addresses; | |
| } catch { | |
| addresses = null; | |
| } | |
| let addresses; | |
| try { | |
| addresses = require('@yellow-org/contracts').addresses; | |
| } catch (err) { | |
| if (err && err.code === 'MODULE_NOT_FOUND') { | |
| addresses = null; | |
| } else { | |
| throw err; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sync-contracts-docs.js` around lines 23 - 28, The try/catch around
require('@yellow-org/contracts').addresses is too broad and hides real errors;
update the try/catch in scripts/sync-contracts-docs.js that assigns addresses so
it only swallows a MODULE_NOT_FOUND error and rethrows any other
exceptions—i.e., after calling require('@yellow-org/contracts') (the expression
used to set addresses), inspect the caught error.code and if it equals
'MODULE_NOT_FOUND' set addresses = null, otherwise throw the error so issues
like syntax or export problems surface.
| for (const [chainIdStr, addrs] of Object.entries(addresses)) { | ||
| const chainId = Number(chainIdStr); | ||
| const chain = CHAINS[chainId]; | ||
| if (!chain) continue; | ||
|
|
There was a problem hiding this comment.
Unknown chain IDs are silently dropped from docs.
if (!chain) continue hides new deployments when the package adds chains not yet mapped in CHAINS. Render them with a fallback label instead of skipping.
Proposed fix
- const chain = CHAINS[chainId];
- if (!chain) continue;
+ const chain = CHAINS[chainId] || { name: `Chain ${chainId}`, explorer: null };
@@
- if (addr) {
- lines.push(`| ${label} | [\`${addr}\`](${chain.explorer}/${addr}) |`);
+ if (addr) {
+ const rendered = chain.explorer ? `[\`${addr}\`](${chain.explorer}/${addr})` : `\`${addr}\``;
+ lines.push(`| ${label} | ${rendered} |`);
} else {
lines.push(`| ${label} | — |`);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/sync-contracts-docs.js` around lines 155 - 159, The loop over
Object.entries(addresses) currently drops unknown chain IDs with if (!chain)
continue; instead create a fallback chain object so unknown chains are rendered
rather than skipped: when CHAINS[chainId] is falsy, set a fallback like { id:
chainId, name: `chain-${chainId}` } (or a descriptive label) and use that
fallback for subsequent rendering logic (replace usages of chain with this
fallbackChain variable); ensure any downstream properties accessed (e.g.,
chain.name or chain.id) are read from the fallback so new deployments appear in
the docs.
1 participant
Summary by CodeRabbit
Documentation
Chores