feat(content): improve support for hierarchical content #2323
Conversation
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This PR touches multiple package scopes: Please confirm the changes are closely related. |
📝 WalkthroughWalkthroughThis pull request addresses nested content directory support and clarifies content system limitations. Changes include: documentation updates explaining that Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Key observationsAPI additions:
These are backward-compatible additions with sensible defaults ( Logic change — slug resolution: Test coverage: Documentation: Monorepo consistency: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
) When a content file lived in a subdirectory (e.g. src/content/docs/erste-schritte/willkommen.md) and its frontmatter slug contained a path separator, the lookup factory rebuilt the path as root-relative (/src/content/<slug>) and dropped the file's own subdirectory. injectContent({ subdirectory: 'docs', param: 'slug' }) then failed to resolve the file because the candidate path included the docs/ prefix the lookup had stripped. Scope slash-containing slugs to the file's top-level subdirectory (e.g. /src/content/docs) instead of the absolute content root. Files directly under /src/content keep the original root-relative behavior since they have no subdirectory of their own. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2318) `contentDir` discovery only globbed top-level files, so prerender configs pointing at directories with category subdirectories silently missed nested content. Add an opt-in `recursive: true` flag on PrerenderContentDir and surface the file's directory relative to `contentDir` as `relativePath` on PrerenderContentFile so transforms can disambiguate identically-named files across subdirectories. Default behavior is unchanged for existing configs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2318) Add a Hierarchical (Nested) Content recipe under the Subdirectories section showing the catch-all + injectContent pattern, including the behavior of slash-containing frontmatter slugs. Note that injectContentFiles returns metadata only and that the content body must be loaded via injectContent. Document the new recursive option on PrerenderContentDir and the relativePath field for disambiguating nested files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c5f3b3e to
27cb05e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/content/src/lib/inject-content-files.ts (1)
9-38:⚠️ Potential issue | 🟠 MajorAdd
BREAKING CHANGE:footer to commit message — required per CONTRIBUTING.md.The type narrowing is correct and fixes a real type-checking issue:
injectContentFiles<T>()now returnsContentFile<T, never>[]where.contentisneverbecause the body is never loaded. This is already documented behavior and no in-repo usages are broken. However, external consumers reading.contentwill face a compile error, making this a public TypeScript API break that requires aBREAKING CHANGE:footer with before/after examples in the commit message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/content/src/lib/inject-content-files.ts` around lines 9 - 38, The commit introduced a public TypeScript API change where injectContentFiles<T>() now returns ContentFile<T, never>[] (making .content typed as never), so update the commit message to include a "BREAKING CHANGE:" footer describing the change, its impact, and before/after examples; specifically mention the injectContentFiles function signature change and how external consumers reading .content will need to adapt, and ensure the footer appears in the commit message body per CONTRIBUTING.md.
🧹 Nitpick comments (2)
packages/content/src/lib/content-files-token.ts (1)
30-40: Subdirectory-scoped slug resolution looks correct.For a normalized
/src/content/<sub>/…/file.md,fileParts.length > 4correctly identifies "lives under a top-level subdirectory of/src/content", andslice(0, 4).join('/')yields exactly/src/content/<sub>. Files directly under/src/contentkeep the old root-relative behavior — which the spec atcontent-files-token.spec.tslines 30-53 pins down. Nice.One tiny nit you can take or leave: the magic number
4is load-bearing on the leading-/(fileParts[0] === ''). A short comment like// fileParts: ['', 'src', 'content', '<sub>', …]next to the> 4check would make it harder to break later. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/content/src/lib/content-files-token.ts` around lines 30 - 40, Add a brief clarifying comment next to the fileParts length check explaining the expected shape of fileParts (e.g. fileParts: ['', 'src', 'content', '<sub>', …]) and why the magic number 4 is used so future editors understand that subdirRoot is computed from slice(0, 4). Reference the symbols subdirRoot, fileParts, newBase, and lookup[contentFilename] so the comment sits beside the existing check: const subdirRoot = fileParts.length > 4 ? fileParts.slice(0, 4).join('/') : '/src/content';.packages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.ts (1)
64-76: Targeted coverage — consider one extra deeper-nesting assertion.The current cases pin top-level (
'') and one-level-deep ('erste-schritte','assets'). A two-level case (e.g.src/content/docs/guides/intro/page.md→relativePath === 'guides/intro') would lock in the path-separator handling for deeper trees without much overhead. Optional in chill mode, but cheap insurance against a future refactor that joins with the wrong separator.♻️ Suggested addition
+ mkdirSync(join(workspaceRoot, 'src/content/docs/guides/intro'), { + recursive: true, + }); + writeFileSync( + join(workspaceRoot, 'src/content/docs/guides/intro/page.md'), + '---\ntitle: Page\n---\n# Page', + ); @@ expect(byName['hochladen'].relativePath).toBe('assets'); + expect(byName['page'].relativePath).toBe('guides/intro');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.ts` around lines 64 - 76, Add a test asserting a two-level nested content file's relativePath to ensure path-separator handling for deeper trees; after calling getMatchingContentFilesWithFrontMatter in the test, extend the byName assertions to include a case like byName['<two-level-file-name>'].relativePath === 'guides/intro' (replace <two-level-file-name> with the fixture name that resides at content/docs/guides/intro/page.md) so the test covers a depth of two directories when using getMatchingContentFilesWithFrontMatter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs-app/docs/features/routing/content.md`:
- Line 277: The change makes injectContentFiles return items whose content
property is typed as never, which is a TypeScript breaking change for consumers
that previously accessed file.content; update the release notes/commit footer
with a "BREAKING CHANGE:" entry and include a short before/after example showing
the previous behavior (accessing file.content) and the recommended workaround
(cast the result to ContentFile<T>[] if you really need to access content, or
use injectContent to load the file body), and add a one-line note in the docs
near injectContentFiles explaining that content is not loaded and is typed never
so callers must cast to ContentFile or use injectContent.
- Line 524: The docs text incorrectly states omitted frontmatter slugs are keyed
by on-disk path; per packages/content/src/lib/content-files-token.ts (logic
around defaulting absent/empty slug to 'index' at lines ~25–29) an empty slug
becomes 'index' before lookup, so update
apps/docs-app/docs/features/routing/content.md to (1) correct the claim to state
that absent/empty slug defaults to 'index' (show a short example mapping
src/content/docs/getting-started/welcome.md -> keyed as
src/content/docs/getting-started/index.md), (2) call out the silent-collision
risk when siblings omit slugs and recommend always setting a frontmatter slug
for nested files (or using explicit index files), and (3) add a brief example
showing how to avoid collisions (e.g., set slug: getting-started/welcome); also
add a unit test in packages/content/src/lib/content-files-token.spec.ts to
assert empty-slug files are mapped to 'index' and that two sibling files without
slugs would collide/overwrite.
In `@packages/content/src/lib/content-files-token.spec.ts`:
- Around line 55-65: The test description says "without a frontmatter slug" but
the fixture supplies slug: 'intro', so either rename the test to reflect that it
checks a non-slash slug for a nested file or change the fixture to use an empty
slug to exercise the filename-derived default behavior; update the spec in
content-files-token.spec.ts by modifying the setup call (the object with
filename '/src/content/docs/intro.md' and slug) so slug: '' if you want to test
the empty-frontmatter → 'index'/filename-derived branch, or change the it(...)
description to something like "preserves explicit filename slugs for nested
files" if you intend to keep slug: 'intro'.
---
Outside diff comments:
In `@packages/content/src/lib/inject-content-files.ts`:
- Around line 9-38: The commit introduced a public TypeScript API change where
injectContentFiles<T>() now returns ContentFile<T, never>[] (making .content
typed as never), so update the commit message to include a "BREAKING CHANGE:"
footer describing the change, its impact, and before/after examples;
specifically mention the injectContentFiles function signature change and how
external consumers reading .content will need to adapt, and ensure the footer
appears in the commit message body per CONTRIBUTING.md.
---
Nitpick comments:
In `@packages/content/src/lib/content-files-token.ts`:
- Around line 30-40: Add a brief clarifying comment next to the fileParts length
check explaining the expected shape of fileParts (e.g. fileParts: ['', 'src',
'content', '<sub>', …]) and why the magic number 4 is used so future editors
understand that subdirRoot is computed from slice(0, 4). Reference the symbols
subdirRoot, fileParts, newBase, and lookup[contentFilename] so the comment sits
beside the existing check: const subdirRoot = fileParts.length > 4 ?
fileParts.slice(0, 4).join('/') : '/src/content';.
In `@packages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.ts`:
- Around line 64-76: Add a test asserting a two-level nested content file's
relativePath to ensure path-separator handling for deeper trees; after calling
getMatchingContentFilesWithFrontMatter in the test, extend the byName assertions
to include a case like byName['<two-level-file-name>'].relativePath ===
'guides/intro' (replace <two-level-file-name> with the fixture name that resides
at content/docs/guides/intro/page.md) so the test covers a depth of two
directories when using getMatchingContentFilesWithFrontMatter.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 813610c7-7fbe-4ae5-a894-08c4db31a2df
📒 Files selected for processing (11)
apps/docs-app/docs/features/routing/content.mdapps/docs-app/docs/features/server/static-site-generation.mdpackages/content/src/lib/content-file.tspackages/content/src/lib/content-files-token.spec.tspackages/content/src/lib/content-files-token.tspackages/content/src/lib/content-locale.tspackages/content/src/lib/inject-content-files.tspackages/vite-plugin-nitro/src/lib/options.tspackages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.tspackages/vite-plugin-nitro/src/lib/utils/get-content-files.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/content/src/lib/content-files-token.spec.ts`:
- Around line 19-66: Add a unit test in the content-files-token.spec.ts that
exercises the slug === '' → 'index' branch: use the existing setup helper
(overriding CONTENT_FILES_LIST_TOKEN and injecting CONTENT_FILES_TOKEN) with a
content file whose filename is nested (e.g., '/src/content/docs/welcome.md') and
slug set to '' and assert the resulting map contains an entry at
'/src/content/docs/index.md' (and optionally assert sibling-collision behavior
by adding a second file in the same folder with slug '' to verify how
CONTENT_FILES_TOKEN handles duplicates). Ensure the new it(...) follows the same
pattern as the other cases and references the same setup helper and map lookups.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0301b060-a9d3-409b-9f10-4875f25d5261
📒 Files selected for processing (8)
apps/docs-app/docs/features/routing/content.mdapps/docs-app/docs/features/server/static-site-generation.mdpackages/content/src/lib/content-files-token.spec.tspackages/content/src/lib/content-files-token.tspackages/vite-plugin-nitro/src/lib/options.tspackages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.tspackages/vite-plugin-nitro/src/lib/utils/get-content-files.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
✅ Files skipped from review due to trivial changes (1)
- apps/docs-app/docs/features/server/static-site-generation.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
- packages/content/src/lib/content-files-token.ts
- packages/vite-plugin-nitro/src/lib/utils/get-content-files.ts
- packages/vite-plugin-nitro/src/lib/utils/get-content-files.spec.ts
- packages/vite-plugin-nitro/src/lib/options.ts
PR Checklist
Closes #2318
Affected scope
Recommended merge strategy for maintainer [optional]
Commit preservation note [optional]
Each commit addresses a distinct part of the issue and keeps the runtime, prerender, and docs changes independently revertible.
What is the new behavior?
injectContentFilescontent body — documentation only. Adds an explicit note in the content guide thatinjectContentFilesreturns metadata (filename,slug,attributes) and that reading.contentyieldsundefined; users should callinjectContentto load a file's body. No type change.subdirectory+ slash-containing slug — slash-containing frontmatter slugs are scoped to the file's top-level subdirectory (e.g./src/content/docs) rather than the absolute content root, soinjectContent({ subdirectory, param })resolves nested files. Files directly under/src/contentkeep the original root-relative behavior.contentDirprerender —PrerenderContentDirgains opt-inrecursive: true, andPrerenderContentFilegainsrelativePathso transforms can disambiguate identically-named files across subdirectories. Default behavior unchanged.injectContentpattern, including the slash-slug behavior; new "Recursing Into Subdirectories" example in the SSG guide.Test plan
nx test content --testPathPattern=content-files-token.spec(3 new tests)nx test vite-plugin-nitro --testPathPattern=get-content-files.spec(3 new tests)content.spec.ts(8 tests) andvite-plugin-nitrosuite (33 tests) greenDoes this PR introduce a breaking change?
recursivedefaults tofalse, so prerender discovery is unchanged for existing configs.Other information
The three commits are independently revertible if any one of them surfaces an issue.