fix(ssr): local content components on Cloudflare Workers#3704
fix(ssr): local content components on Cloudflare Workers#3704
Conversation
|
@onmax is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
📝 WalkthroughWalkthroughRuntime component resolution now reads a generated Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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)
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 |
|
Now, the application built with the
|
![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
🤖 Fix all issues with AI agents
In `@src/utils/templates.ts`:
- Around line 150-151: The generated re-export line in the localComponents
mapping uses a raw single-quoted path which breaks if the path contains a single
quote; change the template generation in templates.ts where localComponents.map
is used (the arrow mapping that produces `export { ${exp} as ${pascalName} }
from '${path}'`) to serialize/escape the path with JSON.stringify(path) so the
emitted import string is properly quoted and escaped; update that mapping
expression to use the JSON.stringify-wrapped path reference.
🧹 Nitpick comments (1)
src/utils/templates.ts (1)
138-144: Introduce a typed tuple for component entries.
Usingunknown[]with positional indices makes the newexportfield fragile and hard to read. A typed tuple clarifies intent and avoids unsafe indexing.♻️ Proposed refactor
+type ComponentEntry = [pascalName: string, path: string, global: boolean, exportName: string] + - }, {} as Record<string, unknown[]>) + }, {} as Record<string, ComponentEntry>)
|
The clean reproduction I provided in the issue is now OK. I wanted to test the fix in my project where I use the To trigger the error, on a clean reproduction without any additional configuration simply add the module in |
|
Before applying the patch, the component imports in the
and after applying the patch, they look like this:
I haven't analyzed this in depth, but won't this have an impact on performance? I also wondered what works differently in the |
|
Dynamic imports inside render functions don't work reliably in CF Workers runtime. Direct imports work, but lose lazy loading benefit.
I would go with the first one, but it is risky since is different behavior per env. |
Co-authored-by: Baptiste Leproux <leproux.baptiste@gmail.com>
…#3709) Co-authored-by: Farnabaz <farnabaz@gmail.com>
|
Changing dynamic import into static import is not ideal solution, it will load all components when We should find a way to sole dynamic import problem. |
|
Ready for review. I added two Cloudflare preview links in the PR body: main (fails SSR) and this branch (SSR fixed). |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/content/docs/7.integrations/01.i18n.md`:
- Around line 89-92: The computed slug currently assumes route.params.slug
exists and will produce "/undefined" for root routes; update the slug computed
(the computed() that references route.params.slug and withLeadingSlash) to
handle undefined by using a safe fallback (e.g., treat undefined as an empty
string) before joining or stringifying; check Array.isArray(route.params.slug)
and if undefined return withLeadingSlash('') or similar so useRoute/useI18n
logic yields "/" instead of "/undefined".
🧹 Nitpick comments (3)
CHANGELOG.md (1)
3-11: Consider noting the Cloudflare Workers SSR/local components fix.This PR’s core change (local content components SSR on Cloudflare Workers) isn’t reflected in the 3.11.1 bug-fix list. Adding a bullet will make the release notes more complete.
📝 Suggested changelog addition
### Bug Fixes * issue with disabling `contentRawMarkdown` ([5be6b0c](https://github.com/nuxt/content/commit/5be6b0cdf5d50440010a988dfd472e69d989f3ef)) +* fix SSR of local content components on Cloudflare Workers (local component loaders)src/utils/config.ts (1)
74-74: Redundant fallback touseNuxt().The
nuxtparameter is a required argument ofloadContentConfig, so it should always be defined. The|| useNuxt()fallback is unnecessary and could cause confusion.Suggested fix
- if (hasNuxtModule('nuxt-studio', nuxt || useNuxt())) { + if (hasNuxtModule('nuxt-studio', nuxt)) {src/utils/studio.ts (1)
52-67: Document the in-place mutation.The function mutates
collectionsConfigdirectly (adding exclude patterns). While this is acceptable given the call site inconfig.ts, consider adding a brief JSDoc note to make the side effect explicit for future maintainers.Suggested documentation enhancement
/** * Resolves studio collection configuration when nuxt-studio is installed. * Automatically creates a studio collection and adds exclude patterns to other collections. + * + * `@remarks` Mutates `collectionsConfig` in place. */ export function resolveStudioCollection(
| const route = useRoute() | ||
| const { locale } = useI18n() | ||
| const slug = computed(() => withLeadingSlash(String(route.params.slug))) | ||
| const slug = computed(() => Array.isArray(route.params.slug) ? withLeadingSlash(String(route.params.slug.join('/'))) : withLeadingSlash(String(route.params.slug))) | ||
There was a problem hiding this comment.
Handle undefined slug to avoid /undefined on root routes.
In catch-all routes, route.params.slug can be undefined (e.g., /). The current snippet would produce /undefined. Consider a safe fallback.
💡 Suggested adjustment
-const slug = computed(() => Array.isArray(route.params.slug) ? withLeadingSlash(String(route.params.slug.join('/'))) : withLeadingSlash(String(route.params.slug)))
+const slug = computed(() => {
+ const raw = route.params.slug
+ const path = Array.isArray(raw) ? raw.join('/') : (raw ?? '')
+ return withLeadingSlash(String(path))
+})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const route = useRoute() | |
| const { locale } = useI18n() | |
| const slug = computed(() => withLeadingSlash(String(route.params.slug))) | |
| const slug = computed(() => Array.isArray(route.params.slug) ? withLeadingSlash(String(route.params.slug.join('/'))) : withLeadingSlash(String(route.params.slug))) | |
| const route = useRoute() | |
| const { locale } = useI18n() | |
| const slug = computed(() => { | |
| const raw = route.params.slug | |
| const path = Array.isArray(raw) ? raw.join('/') : (raw ?? '') | |
| return withLeadingSlash(String(path)) | |
| }) |
🤖 Prompt for AI Agents
In `@docs/content/docs/7.integrations/01.i18n.md` around lines 89 - 92, The
computed slug currently assumes route.params.slug exists and will produce
"/undefined" for root routes; update the slug computed (the computed() that
references route.params.slug and withLeadingSlash) to handle undefined by using
a safe fallback (e.g., treat undefined as an empty string) before joining or
stringifying; check Array.isArray(route.params.slug) and if undefined return
withLeadingSlash('') or similar so useRoute/useI18n logic yields "/" instead of
"/undefined".
9 participants
Summary
localComponentLoaders.import('#content/components')inContentRenderer.Why
Cloudflare Workers SSR fails to render local MDC components when
#content/componentsis dynamically imported. Loading local components via per-component async loaders avoids the Worker import issue while keeping SSR intact.Previews (repro on same worker)
@nuxt/content@3.11.0) fails SSR for local components: previewSSR: Component rendered on server.SSR: Component rendered on server.Tests
@nuxtjs/seoenabled (no missing-export errors).