Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
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
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.
docs/ensnode.io/src/components/overrides/TwoColumnContent.astro
Outdated
Show resolved
Hide resolved
| 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
There was a problem hiding this comment.
I looked to implement this last week but there is a lot more involved that would require changing the CI steps so we output the spec to the public directory. We could change the output directory of the generated spec (and what our CI check looks at), or copy on build to the public folder so we can use the url param of ScalarComponent. Both approaches are fine in my opinion.
I'm more inclined to follow option A to move the output directory to public so that it can be served from the docs site too, but I would do this in a separate PR. Which I can do next week @lightwalker-eth
|
lgtm |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tableOfContents: | ||
| minHeadingLevel: 6 | ||
| maxHeadingLevel: 6 |
There was a problem hiding this comment.
PR description mentions the API reference page uses tableOfContents: false, but this frontmatter sets tableOfContents to an options object (min/max heading level 6). If the intent is to disable Starlight TOC for this page, set it explicitly to false; if the object form is intentional as a workaround, please align the PR description (or add a short comment here explaining why).
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.