Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes integrate Scalar API reference functionality into the Starlight documentation, adding new layout components to replace existing overrides, introducing an interactive API reference page, and adjusting styling to accommodate the restructured layout. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Config as Starlight Config
participant Layout as Layout Overrides
participant PageSidebar as PageSidebar
participant TwoColumnContent as TwoColumnContent
participant API as ScalarComponent
User->>Config: Navigates to Reference page
Config->>Layout: Activates PageSidebar & TwoColumnContent overrides
Layout->>PageSidebar: Initializes with starlightRoute
PageSidebar->>PageSidebar: Render TOC (conditional) + TelegramInvite
Layout->>TwoColumnContent: Manages two-column layout structure
TwoColumnContent->>TwoColumnContent: Apply fixed sidebar positioning
TwoColumnContent->>API: Right sidebar slot contains main content
API->>API: Render OpenAPI spec with custom config
API-->>User: Display interactive API reference
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Pull request overview
Adds a Scalar-based OpenAPI reference experience to the docs/ensnode.io Astro/Starlight docs site, including a new “separate” API reference page and the required dependencies.
Changes:
- Add Scalar API Reference dependencies (Scalar + Vue) to the docs app and update the pnpm lockfile accordingly.
- Introduce a new standalone Astro page rendering Scalar’s API reference from the generated
ensapi-openapi.json. - Add an MDX-based reference page and wire “Reference” and “API Reference (Separate)” into the Starlight sidebar.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile for Scalar/Vue additions and postcss-related transitive changes. |
| docs/ensnode.io/package.json | Adds @scalar/* packages and Vue dependencies needed by Scalar. |
| docs/ensnode.io/config/integrations/starlight.ts | Adds ENSApi “Reference” section and a separate API reference link in the sidebar. |
| docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro | New standalone Scalar API reference page with custom header. |
| docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx | New MDX page using @scalar/astro’s ScalarComponent. |
| docs/ensnode.io/src/components/molecules/StaticHeader.astro | Minor formatting changes in header markup (touched region includes a Tailwind class issue). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ensnode.io/package.json`:
- Around line 29-33: The astro.config.mjs is missing the `@astrojs/vue`
integration required for SSR of Vue components; open astro.config.mjs, import
the Vue integration (e.g., import vue from '@astrojs/vue') and add vue() to the
existing integrations array (next to mermaid(), starlight(), sitemap(), react(),
mdx(), icon()) so it becomes integrations: [mermaid(), starlight(), sitemap(),
react(), mdx(), icon(), vue()], ensuring Vue-based components (used by
`@scalar/astro`) are properly server-rendered.
In `@docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx`:
- Around line 11-33: The ScalarComponent configuration contains conflicting UI
settings: keep the desired UX consistent by either enabling download controls or
removing download type, and by restoring navigation if you want endpoint
discoverability; specifically, in the ScalarComponent props (configuration
object) reconcile hideDownloadButton and documentDownloadType by either setting
hideDownloadButton: false when you want downloads (and keep
documentDownloadType: "both") or remove/clear documentDownloadType when
downloads are intentionally hidden, and similarly decide whether to provide
navigation by setting showSidebar: true if you need endpoint navigation (and/or
set hideSearch: false) or leave both off for a minimal embedded view.
- Around line 1-6: Update the frontmatter "description" value in this document
(the YAML block containing title: ENSApi API Reference and sidebar:) to
accurately describe the page purpose; replace the current "Learn how to run
ENSApi locally for development and contributions." with a succinct API-focused
description such as "Interactive API reference documentation for ENSApi
endpoints." so the description matches the API reference content.
In `@docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro`:
- Around line 74-75: The code uses non-null assertions for
document.getElementById("scalar-api-reference") and el.dataset.config which can
throw at runtime; update the logic around the const el and const config
declarations to guard against missing DOM or dataset by checking that
document.getElementById("scalar-api-reference") returned an element and that
el.dataset.config is defined before calling JSON.parse, and handle failure paths
(e.g., log/error and bail out or provide a default config) and wrap JSON.parse
in try/catch to surface parse errors; reference the "el" and "config" variables
and the "scalar-api-reference" id when making these checks.
- Line 69: The JSX/HTML element rendering the scalar API reference has a
malformed attribute sequence—add a space between the id attribute and the
data-config attribute on the element that contains id="scalar-api-reference" and
data-config={scalarConfig} so it becomes two separate attributes
(id="scalar-api-reference" and data-config={scalarConfig}) to produce valid
markup.
- Line 51: The Tailwind class string in the div element contains a typo:
`lg::mr-[76px]` uses a double colon; update the class value in the element (the
div with classes "flex flex-row justify-between items-center gap-2 sm:gap-[14px]
cursor-pointer flex-shrink-0 sm:mr-10 lg::mr-[76px]") to use a single breakpoint
colon `lg:mr-[76px]` so Tailwind applies the large-screen right margin
correctly.
- Around line 48-63: Replace the duplicated header block with the existing
StaticHeader component: remove the inline <header>... block and import/use
StaticHeader (ensure HeaderButtons and ENSNode2DLight usage is preserved or
passed as props if StaticHeader expects them); while doing so, fix the typo
"lg::mr-[76px]" to "lg:mr-[76px]" either in this file before replacing or inside
StaticHeader so the spacing is correct; verify StaticHeader's layout (and
whether CustomSearch is intentionally omitted) and adjust props or composition
so the visual/behavioral output remains identical.
🪄 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: 6c4ca732-eff5-4c13-a345-6457bbf9f08f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
docs/ensnode.io/config/integrations/starlight.tsdocs/ensnode.io/package.jsondocs/ensnode.io/src/components/molecules/StaticHeader.astrodocs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdxdocs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
docs/ensnode.io/src/pages/ensapi/reference/separate-api-reference.astro
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR integrates the interactive Scalar API reference widget into the ENSNode Starlight documentation site by embedding Key changes:
Confidence Score: 4/5Safe to merge after verifying the tailwindcss catalog downgrade is intentional and scoping the generic .section customCss rule. Two P1 findings prevent a clean 5: (1) the .section selector in Scalar's customCss is injected globally and will zero out padding-inline on any other .section element on the page, and (2) adding the Scalar packages caused the workspace-wide tailwindcss catalog to regress from 4.2.2 to 4.1.18, which could silently break utilities elsewhere in the monorepo. All other changes are well-reasoned and low-risk. docs/ensnode.io/src/content/docs/ensapi/reference/api-reference.mdx (customCss selector scope) and pnpm-lock.yaml (tailwindcss version regression) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Starlight Page Request] --> B{tableOfContents enabled?}
B -- Yes --> C[TwoColumnContent renders:
right-sidebar slot = TOC
below = TelegramInvite]
B -- No
(api-reference page) --> D[TwoColumnContent renders:
right-sidebar slot = empty
below = TelegramInvite only]
C --> E[main-pane: page content]
D --> F[main-pane: ScalarComponent
with ENSApi OpenAPI spec]
F --> G{User clicks Test Request?}
G -- Yes --> H[Modal opens — stacking context
not trapped because isolation:isolate
was removed from .main-pane]
|
| documentDownloadType: "both", | ||
| hideTestRequestButton: false, |
There was a problem hiding this comment.
Contradictory download configuration
hideDownloadButton: true suppresses the download button entirely, making documentDownloadType: "both" dead configuration. The download-type setting has no effect when the button isn't rendered. Consider removing it to keep the configuration tidy.
| documentDownloadType: "both", | |
| hideTestRequestButton: false, | |
| hideDownloadButton: true, |
| hideDarkModeToggle: true, | ||
| withDefaultFonts: true, | ||
| expandAllModelSections: true, | ||
| customCss: `.scalar-mcp-layer { display: none !important; } .section { padding-inline: 0 !important; }`, |
There was a problem hiding this comment.
Generic
.section selector may leak into the Starlight page
Scalar's customCss is injected as a global <style> tag (not scoped to the Scalar component), so .section { padding-inline: 0 !important; } will match any element with class section on the full page — not just inside the API reference widget. If Starlight or any other component uses a .section class, its padding-inline will be zeroed out silently.
Consider scoping the rule to the Scalar root to limit the blast radius:
| customCss: `.scalar-mcp-layer { display: none !important; } .section { padding-inline: 0 !important; }`, | |
| customCss: `.scalar-mcp-layer { display: none !important; } .scalar-app .section { padding-inline: 0 !important; }`, |
| showDeveloperTools: "never", | ||
| isEditable: false, | ||
| isLoading: false, | ||
| documentDownloadType: "both", |
| hideDarkModeToggle: true, | ||
| withDefaultFonts: true, | ||
| expandAllModelSections: true, | ||
| customCss: `.scalar-mcp-layer { display: none !important; } .section { padding-inline: 0 !important; }`, |
There was a problem hiding this comment.
| customCss: `.scalar-mcp-layer { display: none !important; } .section { padding-inline: 0 !important; }`, | |
| customCss: `.scalar-mcp-layer { display: none !important; } .scalar-api-reference .section { padding-inline: 0 !important; }`, |
Generic .section CSS selector in customCss causes style leakage affecting all elements with class="section" on the page, not just within the Scalar component
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@namehash/namekit-react": "catalog:", | ||
| "@namehash/namehash-ui": "workspace:*", | ||
| "@octokit/rest": "^20.1.2", | ||
| "@scalar/api-reference": "^1.49.3", |
There was a problem hiding this comment.
@scalar/api-reference is added as a direct dependency but isn’t imported/used anywhere in this docs app (only @scalar/astro is referenced). If it’s not required as a runtime peer for @scalar/astro, removing it would significantly reduce install size and transitive dependencies; if it is required, consider adding an inline comment explaining why it must be direct.
| "@scalar/api-reference": "^1.49.3", |
| /** | ||
| * Override of Starlight's TwoColumnContent that always renders the right | ||
| * sidebar column. The original conditionally hides it when tableOfContents | ||
| * is disabled, which also removes our TelegramInvite. |
There was a problem hiding this comment.
This override is a forked/copy of Starlight’s internal TwoColumnContent, which will likely need manual syncing on Starlight upgrades. To make future updates safer, please include the upstream Starlight version (and ideally a link/path to the original source) in this header comment so reviewers know what it was copied from.
| * is disabled, which also removes our TelegramInvite. | |
| * is disabled, which also removes our TelegramInvite. | |
| * | |
| * Forked from Starlight's internal `TwoColumnContent` component. | |
| * Starlight upstream version at time of copy: TODO-update-with-exact-version (e.g. v0.XX.X). | |
| * Upstream source path (for reference when syncing): see Starlight's internal `TwoColumnContent` | |
| * component in the Starlight repo (packages/starlight/.../TwoColumnContent.astro or equivalent). |
There was a problem hiding this comment.
I think this is a fair tradeoff. It hasn't been updated much from what I can tell, and the page looks better without the single list item in the TOC.
| import openapiSpec from "@workspace/docs/docs.ensnode.io/ensapi-openapi.json"; | ||
|
|
||
| <ScalarComponent | ||
| configuration={{ | ||
| content: openapiSpec, |
There was a problem hiding this comment.
Importing the full OpenAPI JSON at build time will bundle the entire spec into the page’s JS payload. If Scalar supports loading from a URL or static asset path, consider switching to that approach to reduce client bundle size and improve initial load performance (especially as the spec grows).
| import openapiSpec from "@workspace/docs/docs.ensnode.io/ensapi-openapi.json"; | |
| <ScalarComponent | |
| configuration={{ | |
| content: openapiSpec, | |
| <ScalarComponent | |
| configuration={{ | |
| spec: { url: "/ensapi-openapi.json" }, |
There was a problem hiding this comment.
Doing so would (URL) mean we can't view in our preview deployments the upcoming API spec changes. Need to investigate the static asset path though... I assume this means just putting it in the public directory, instead of where it is now in the root.
There was a problem hiding this comment.
i think not worth optimizing, is a-ok
|
lgtm |
Summary
TwoColumnContentto always render the right sidebar column and removeisolation: isolatefrom.main-paneWhy
Embedding the Scalar API reference directly in Starlight provides an integrated documentation experience.
How it was tested
tableOfContentsenabled still display the TOC and TelegramInvite correctlytableOfContents: false) shows TelegramInvite in the right sidebarNotes
TwoColumnContentoverride is a near-copy of Starlight's original with three changestoc &&guard on the<aside>isolation: isolatefrom.main-pane[data-has-sidebar][data-has-toc]to[data-has-sidebar]in the width selector. These are all internal to the component and can't be achieved by wrapping the default.TwoColumnContentupstream, this override may need to be synced..scalar-app .z-overlay(descendant selector) for the modal's z-index, but both classes are on the same element. This is a Scalar bug, but removingisolation: isolatemakes it moot since the modal's stacking context is no longer trapped.