fix: Include Render partial headings in page TOC - #32267
Conversation
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
Preview URL: https://b02c6b8a.preview.developers.cloudflare.com |
Review
👉 Fix in your agent 👈Fix the following review findings in PR #32267 (https://github.com/cloudflare/cloudflare-docs/pull/32267).
Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order
After triaging, follow this order:
1. Post a comment on this PR for any findings you are skipping, with the finding ID and your reasoning.
2. Then commit the fixes for the legitimate findings.
The comment must come before the commit — the bot reads PR comments when a new
push triggers a review, so skip comments posted after the push will be missed.
---
## Code Review
### Warnings (1)
#### CR-63820172e021 · Regex cannot match multiline JSX attributes
- **File:** `src/util/rendered-toc.ts` line 29
- **Issue:** The file/product attribute regexes on lines 29–30 use `.`, which does not match newline characters. `<Render>` usage in the repo is commonly multiline, so `file`/`product` attributes that follow a newline will not be captured and referenced dynamic partials will be missed.
- **Fix:** Add the `s` flag to the attribute regexes (e.g. `/\bfile=["']([^"']+)["']/s`) so `.` matches newlines, or use `[^]*?`/single capture when parsing the whole tag.
Code ReviewThis code review is in beta and may not always be helpful — use your judgment. Warnings (1)
ConventionsNo convention issues found. Style Guide ReviewNo style-guide issues found. CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
|
/rebase |
6e56e56 to
42c2e4a
Compare
|
/rebase |
42c2e4a to
e91c3c0
Compare
Triage: findings skippedCR-5d3e0f3f11de & CR-343fe494b089 — Race condition in lazy singleton initSkipping. CR-c5a92a1bb422 — Direct tarball bypasses internal registry gatewaySkipping. The tarball URL is intentional and documented in the comment above the override. The internal registry gateway does not mirror |
Update: override removedCR-0481180a08c6 and CR-c5a92a1bb422 are now moot — the entire |
|
/rebase |
39cb4ad to
b02c6b8
Compare
Summary
Consumes the pkg.pr.new Nimbus build from cloudflare/nimbus#36, which adds
partialHeadingssupport togetDocsPageProps.Adds the cloudflare-docs-specific
resolvePartialIdresolver tosrc/pages/[...slug].astro, matchingRender.astro'sproduct ?${product}/${file}: fileconvention. This lets Nimbus collect markdown headings from the same partials that render in-page via<Render>.Adds
src/util/rendered-toc.ts, a utility that detects when a page (or its transitive<Render>partials) usesAnchorHeading— a component that emits headings at runtime viaset:html, making them invisible to compile-time heading extraction. When runtime headings are detected, the utility renders the page through anAstroContainerto scrape the actual<h*>elements from the rendered HTML. It also transitively resolves partial-to-partial<Render>references to determine which partials are "dynamic" (containAnchorHeadingdirectly or through a dependency chain).Fixes literal markdown headings inside rendered partials missing from the page TOC. For example,
/bots/plans/pro/now includes "Bot settings versus custom rules" (frompartials/bots/bot-settings-vs-custom-rules.mdx) in its on-this-page TOC.Does not fix
AnchorHeading-generated headings; those are runtime component output and need a separate cloudflare-docs-specific follow-up if desired.Documentation checklist