feat: support adding api export entries automatically#271
Conversation
🦋 Changeset detectedLatest commit: f5e1bae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
e8ee9df to
3335873
Compare
WalkthroughThis PR adds automatic discovery and merging of API export entries from the filesystem, updates export configuration (rename + removal), removes the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as load-config()
participant FS as Filesystem
participant CFG as commonConfig / doom.config.yml
participant THEME as Layout Component
CLI->>CFG: read config + commonConfig
CFG-->>CLI: return commonConfig, export_ flag
alt export_ enabled
CLI->>CLI: set local root, lang from commonConfig
CLI->>FS: glob scan for API docs under locale-aware scope (*/apis/**)
FS-->>CLI: discovered apiExports
CLI->>CLI: normalize scopes, compute flattenScope, merge apiExports with config.export
CLI->>CFG: update config.exports with merged list
end
CLI->>THEME: provide final export items
THEME->>THEME: check exportItem.flattenScope?.includes(sidebar._fileKey)
THEME-->>THEME: render sidebar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
commit: |
There was a problem hiding this comment.
Pull request overview
Adds automatic generation of PDF export entries for API docs during “exporting PDF mode”, reducing the need to maintain manual export: config entries.
Changes:
- Auto-detect API doc entrypoints (
apis/{advanced_apis,crds,kubernetes_apis}/*/index.{md,mdx}) and append correspondingexportitems during config load. - Adjust warning suppression regex for rspack to also ignore
vscode-languageserver-types. - Update config/export-related behavior and config entries (including removing a manual API export entry).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/doom/src/theme/Layout.tsx | Avoids crashing when flattenScope is absent by using optional chaining. |
| packages/doom/src/cli/load-config.ts | Adds automatic API export entry generation and tweaks config merge/export handling. |
| packages/doom/package.json | Removes shiki dependency. |
| doom.config.yml | Updates export entries to rely on new automatic API export generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
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 `@packages/doom/src/cli/load-config.ts`:
- Around line 486-512: The code only populates flattenScope when export_ is
true, which leaves mergedConfig.export items without flattened scopes during
normal runs (breaking PDF/link matching); update the logic so that flattenScope
is computed for export items regardless of export_—e.g., after building
mergedConfig.export (or always when config.export exists), map over each
ExportItem (reference: mergedConfig.export, ExportItem, flattenScope) and run
glob(item.scope, { cwd: root }).filter(isDoc) to set flattenScope (ensuring
item.scope is normalized to an array first), rather than doing that mapping only
inside the if (export_) block.
- Around line 497-499: The auto-generated export name using
path.basename(path.dirname(entry)) + '_apis' can collide across categories;
change the name generation in load-config.ts so it uses a sanitized, unique form
of scope instead of just the leaf folder: compute the scope used for scope (the
hasLocales ? scope.replace(lang, '*') : scope) then replace path separators
(/[\/\\]/g) with underscores (and optionally normalize '*' to 'all' or similar)
and append '_apis' so names become e.g. "kubernetes_event_apis" rather than just
"event_apis"; update the assignment to the name property (where name is set) to
use this sanitized scope-based value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04a51925-2019-4aa6-82c1-670530cabbaf
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.changeset/fine-stars-glow.mddoom.config.ymlpackages/doom/package.jsonpackages/doom/src/cli/load-config.tspackages/doom/src/theme/Layout.tsx
💤 Files with no reviewable changes (1)
- packages/doom/package.json
3335873 to
f5e1bae
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/doom/src/cli/load-config.ts (2)
486-512:⚠️ Potential issue | 🟠 Major
flattenScopenormalization is incorrectly gated by export mode.With this guard, non-export runs keep
config.exportentries without normalizedscopearrays and withoutflattenScope, which can break runtime matching paths that rely on these fields (for example,packages/doom/src/theme/Layout.tsxLines 40-81).Suggested fix
- if (export_) { - const hasLocales = !!commonConfig.themeConfig!.locales?.length - - const apiEntries = await glob( - `${hasLocales ? commonConfig.lang! + '/' : ''}apis/{advanced_apis,crds,kubernetes_apis}/*/index.{md,mdx}`, - { cwd: root }, - ) - - const apiExports = apiEntries.map((entry) => { - const scope = entry.replace(/index\.mdx?$/, '') - return { - name: path.basename(path.dirname(entry)) + '_apis', - scope: hasLocales ? scope.replace(lang, '*') : scope, - } - }) - - mergedConfig.export = await Promise.all( - [...(config.export ?? []), ...apiExports].map( - async (item) => - ({ - ...item, - scope: Array.isArray(item.scope) ? item.scope : [item.scope], - flattenScope: (await glob(item.scope, { cwd: root })).filter(isDoc), - }) satisfies ExportItem, - ), - ) - } + const hasLocales = !!commonConfig.themeConfig!.locales?.length + const apiExports = export_ + ? ( + await glob( + `${hasLocales ? commonConfig.lang! + '/' : ''}apis/{advanced_apis,crds,kubernetes_apis}/*/index.{md,mdx}`, + { cwd: root }, + ) + ).map((entry) => { + const scope = entry.replace(/index\.mdx?$/, '') + return { + name: path.basename(path.dirname(entry)) + '_apis', + scope: hasLocales ? scope.replace(lang, '*') : scope, + } + }) + : [] + + if ((config.export?.length ?? 0) || apiExports.length) { + mergedConfig.export = await Promise.all( + [...(config.export ?? []), ...apiExports].map(async (item) => ({ + ...item, + scope: Array.isArray(item.scope) ? item.scope : [item.scope], + flattenScope: (await glob(item.scope, { cwd: root })).filter(isDoc), + })) as Promise<ExportItem>[], + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/doom/src/cli/load-config.ts` around lines 486 - 512, The current logic only normalizes export entries (converting item.scope to arrays and computing flattenScope) when export_ is true, leaving config.export entries unnormalized for normal runs; update load-config so that mergedConfig.export (or another normalized export list) is computed regardless of export_ by mapping over [...(config.export ?? []), ...apiExports] and normalizing each item (ensure scope is Array.isArray => array, compute flattenScope via (await glob(item.scope, { cwd: root })).filter(isDoc)) and preserving the ExportItem shape; reference the existing symbols mergedConfig.export, config.export, export_, flattenScope, ExportItem, glob and isDoc when making the change so runtime consumers like Layout.tsx receive normalized scopes even when not in export mode.
497-499:⚠️ Potential issue | 🟠 MajorAuto-generated API export names can collide.
name: path.basename(path.dirname(entry)) + '_apis'is not unique across categories (e.g.,advanced_apis/eventandkubernetes_apis/event), which risks output overwrites.Suggested fix
- return { - name: path.basename(path.dirname(entry)) + '_apis', + const category = path.basename(path.dirname(path.dirname(entry))) + const leaf = path.basename(path.dirname(entry)) + return { + name: `${category}_${leaf}_apis`, scope: hasLocales ? scope.replace(lang, '*') : scope, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/doom/src/cli/load-config.ts` around lines 497 - 499, The generated export name using name: path.basename(path.dirname(entry)) + '_apis' can collide across categories; update the name generation to incorporate a unique identifier derived from the entry or scope (for example combine path.basename(path.dirname(entry)) with a sanitized version of scope or a short hash of entry) so names like `${path.basename(path.dirname(entry))}_${sanitizeScope(scope)}_apis` (or `${path.basename(path.dirname(entry))}_${shortHash(entry)}_apis`) are produced; modify the code that sets name (and reuse scope.replace(lang, '*') when hasLocales is true) so the final export name is deterministic and unique across categories.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/doom/src/cli/load-config.ts`:
- Around line 486-512: The current logic only normalizes export entries
(converting item.scope to arrays and computing flattenScope) when export_ is
true, leaving config.export entries unnormalized for normal runs; update
load-config so that mergedConfig.export (or another normalized export list) is
computed regardless of export_ by mapping over [...(config.export ?? []),
...apiExports] and normalizing each item (ensure scope is Array.isArray =>
array, compute flattenScope via (await glob(item.scope, { cwd: root
})).filter(isDoc)) and preserving the ExportItem shape; reference the existing
symbols mergedConfig.export, config.export, export_, flattenScope, ExportItem,
glob and isDoc when making the change so runtime consumers like Layout.tsx
receive normalized scopes even when not in export mode.
- Around line 497-499: The generated export name using name:
path.basename(path.dirname(entry)) + '_apis' can collide across categories;
update the name generation to incorporate a unique identifier derived from the
entry or scope (for example combine path.basename(path.dirname(entry)) with a
sanitized version of scope or a short hash of entry) so names like
`${path.basename(path.dirname(entry))}_${sanitizeScope(scope)}_apis` (or
`${path.basename(path.dirname(entry))}_${shortHash(entry)}_apis`) are produced;
modify the code that sets name (and reuse scope.replace(lang, '*') when
hasLocales is true) so the final export name is deterministic and unique across
categories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4277994-aee8-4a58-8d40-247a96aa3579
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.changeset/fine-stars-glow.mddoom.config.ymlpackages/doom/package.jsonpackages/doom/src/cli/load-config.tspackages/doom/src/theme/Layout.tsxpackages/doom/styles/global.scss
💤 Files with no reviewable changes (1)
- packages/doom/package.json
✅ Files skipped from review due to trivial changes (1)
- .changeset/fine-stars-glow.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/doom/src/theme/Layout.tsx
- doom.config.yml
close IDP-1440
Summary by CodeRabbit
New Features
Improvements
Chores
Bug Fixes