fix(content): resolve content file resources during prerender#2248
fix(content): resolve content file resources during prerender#2248benpsnyder wants to merge 10 commits intoanalogjs:alphafrom
Conversation
✅ Deploy Preview for analog-app 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. |
|
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:
📝 WalkthroughWalkthroughA Playwright test in Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/resources/src/content-file-resource.ts`:
- Line 15: The resource currently bypasses the public loader hook by directly
calling injectContentFilesMap(); change content-file-resource.ts to first
attempt the published hook (injectContentFileLoader / CONTENT_FILE_LOADER /
withContentFileLoader) via dependency injection and, if a loader is provided,
call that loader to obtain the files map; only fall back to
injectContentFilesMap() when no loader override is present. Update the logic in
the function/class that currently uses injectContentFilesMap() so it respects
the injected loader symbol names (CONTENT_FILE_LOADER, injectContentFileLoader,
withContentFileLoader) to preserve the override surface.
🪄 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: 157dc76e-6de3-4c75-a12f-9103bb1a29fd
📒 Files selected for processing (1)
packages/content/resources/src/content-file-resource.ts
brandonroberts
left a comment
brandonroberts
left a comment
There was a problem hiding this comment.
Blocked as this removes the ability to resolve content async
…d clean up unused entries
|
Updated this branch to address the blocker from your review.
I also added targeted coverage for both paths:
The existing inline CodeRabbit thread is already resolved on the PR, so I think this should clear the blocker now. |
…S to exports for better accessibility
|
This PR touches multiple package scopes: Please confirm the changes are closely related. |
…ource-prerender-hydration
2 participants
PR Checklist
Closes contentFileResource fails during prerendering hydration analogjs/analog#2165
Affected scope
contentRecommended merge strategy for maintainer [optional]
Commit preservation note [optional]
What is the new behavior?
contentFileResource()keeps the prerender fix without removing async content resolution.When a content file loader override is registered through the public
CONTENT_FILE_LOADER/injectContentFileLoader()/withContentFileLoader()surface, the resource now uses that loader to resolve the files map. When no loader override is present, it falls back toinjectContentFilesMap()so prerendered markdown routes still avoid the initialNo Content Foundpath.That preserves the published override surface for custom loaders while keeping the blog app prerender path working.
Test plan
nx format:checkpnpm buildpnpm testManual verification performed:
pnpm exec vitest run --config packages/content/vite.config.ts packages/content/resources/src/content-file-resource.spec.tscontentFileResource()respects both a custom asyncCONTENT_FILE_LOADERoverride and the defaultwithContentFileLoader()provider in targeted resource testspnpm exec vite build --config apps/blog-app/vite.config.tsdist/apps/blog-app/analog/public/blog/2022-12-27-my-first-post.htmlcontainsMy First Postand rendered markdown instead ofNo Content FoundMy First Postin the browserDoes this PR introduce a breaking change?
Other information
The blocker on this PR was that the earlier implementation removed the async loader path. The current branch restores that compatibility by routing through the published loader hook first and only using the direct content files map fallback when no loader override is registered.