feat: prerender hint based on ssrContext access #184
Conversation
commit: |
📝 WalkthroughWalkthroughThis pull request introduces prerender hint detection and flagging functionality to the Nuxt module. It adds a transform plugin that rewrites Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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: 3
🤖 Fix all issues with AI agents
In `@src/plugins/prerender.ts`:
- Around line 21-22: The naive use of s.replaceAll('import.meta.server',
'(__tryUseNuxtApp()?.ssrContext && import.meta.server)') can match inside
strings/comments and changes the return type; update the transform to only
target real AST occurrences of import.meta.server (e.g., parse the source and
replace MemberExpression nodes) or use a regex that excludes string/comment
contexts, and ensure the replacement coerces to a boolean (e.g., wrap the
expression with !!) so code checking === true keeps working; reference the
s.replaceAll call, the import.meta.server node, and the
__tryUseNuxtApp()?.ssrContext expression when making the change.
In `@src/runtime/prerender/plugin.server.ts`:
- Around line 38-44: The function isUserLandCode misclassifies an undefined
stack line as user-land; modify isUserLandCode so it first checks that the
selected line exists (from getStackTraceLines and the local variable line) and
returns false if line is undefined or empty, then only evaluate
!line.includes('node_modules') && !line.includes('node:internal') to determine
user-land; locate function isUserLandCode and add an explicit existence check
for line before applying includes.
In `@src/runtime/prerender/utils.ts`:
- Around line 1-4: Fix the typo in the top comment block that begins with "Get
all stacktrace lines without the current file" by replacing "hiints" with
"hints" so the comment reads correctly; update that docstring/comment text in
the utils file (the block containing "Don't use in build files if we ever goes
into build mode nuxt hiints. Thank you.") to use "hints".
🧹 Nitpick comments (3)
src/plugins/prerender.ts (1)
15-18: Redundant node_modules check in handler.The filter configuration at lines 11-13 already excludes
node_modulesvia theid.excludepattern. The additional check at line 16 is redundant but harmless.🧹 Simplify by removing redundant check
handler(code, id) { - if (id.includes('node_modules') || !code.includes('import.meta.server')) { + if (!code.includes('import.meta.server')) { return null }src/runtime/prerender/utils.ts (1)
5-13: Consider adding a defensive check forError.captureStackTrace.
Error.captureStackTraceis a V8-specific API. While this server-side code will run on Node.js (V8-based), adding a fallback ensures robustness if the runtime environment changes or for edge cases.🛡️ Suggested defensive implementation
export function getStackTraceLines(): string[] { + if (typeof Error.captureStackTrace !== 'function') { + // Fallback for non-V8 environments + return new Error().stack?.split('\n').slice(1).map(line => line.trim()) ?? [] + } const stackObject: { stack: string } = {} as { stack: string } Error.captureStackTrace(stackObject) return stackObject.stack .split('\n') .slice(1) .map(line => line.trim()) }src/runtime/prerender/plugin.server.ts (1)
13-24: Consider adding property descriptor options for robustness.The
Object.definePropertycall doesn't specifyconfigurableorenumerable, which default tofalse. This prevents other code from modifying or iterating overssrContext. If this is intentional (to protect the interception), consider adding a comment. Otherwise, explicitly set these options.💡 Suggested improvement
Object.defineProperty(nuxtApp, 'ssrContext', { + configurable: true, + enumerable: true, get() { if (watching && isUserLandCode()) { event.context.shouldPrerender = false } return ssrContext }, })
src/plugins/prerender.ts
Outdated
| // ssrContext access will trigger prerender flagging | ||
| s.replaceAll('import.meta.server', '(__tryUseNuxtApp()?.ssrContext && import.meta.server)') |
There was a problem hiding this comment.
Naive string replacement may cause unintended matches.
Using replaceAll on raw source code can match occurrences inside string literals, template literals, or comments (e.g., "import.meta.server" or // import.meta.server). Consider using an AST-based approach or a more precise regex that excludes these contexts.
Additionally, the replacement changes the expression's return type: import.meta.server returns a strict boolean, but (__tryUseNuxtApp()?.ssrContext && import.meta.server) can return the ssrContext object (truthy) or undefined (falsy) instead of true/false. This could break code that uses strict equality checks like === true.
💡 Suggested improvement for stricter replacement
Consider using a regex that better targets actual code usage, or use an AST parser like acorn or recast for precise transformations:
- s.replaceAll('import.meta.server', '(__tryUseNuxtApp()?.ssrContext && import.meta.server)')
+ // Use regex to avoid matching inside strings/comments (basic approach)
+ s.replace(/\bimport\.meta\.server\b/g, '(!!(__tryUseNuxtApp()?.ssrContext) && import.meta.server)')The !! coercion ensures a boolean result is maintained.
📝 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.
| // ssrContext access will trigger prerender flagging | |
| s.replaceAll('import.meta.server', '(__tryUseNuxtApp()?.ssrContext && import.meta.server)') | |
| // ssrContext access will trigger prerender flagging | |
| // Use regex to avoid matching inside strings/comments (basic approach) | |
| s.replace(/\bimport\.meta\.server\b/g, '(!!(__tryUseNuxtApp()?.ssrContext) && import.meta.server)') |
🤖 Prompt for AI Agents
In `@src/plugins/prerender.ts` around lines 21 - 22, The naive use of
s.replaceAll('import.meta.server', '(__tryUseNuxtApp()?.ssrContext &&
import.meta.server)') can match inside strings/comments and changes the return
type; update the transform to only target real AST occurrences of
import.meta.server (e.g., parse the source and replace MemberExpression nodes)
or use a regex that excludes string/comment contexts, and ensure the replacement
coerces to a boolean (e.g., wrap the expression with !!) so code checking ===
true keeps working; reference the s.replaceAll call, the import.meta.server
node, and the __tryUseNuxtApp()?.ssrContext expression when making the change.
| function isUserLandCode(offset: number = 1): boolean { | ||
| const stack = getStackTraceLines() | ||
| const lines = stack.slice(2) | ||
| const line = lines[offset] | ||
| const isUserLand = !line?.includes('node_modules') && !line?.includes('node:internal') | ||
| return isUserLand | ||
| } |
There was a problem hiding this comment.
Bug: undefined line is incorrectly classified as user-land code.
If line is undefined (e.g., when the stack is shorter than expected), the expression !line?.includes('node_modules') && !line?.includes('node:internal') evaluates to !undefined && !undefined = true && true = true, incorrectly marking the code as user-land.
🐛 Proposed fix
function isUserLandCode(offset: number = 1): boolean {
const stack = getStackTraceLines()
const lines = stack.slice(2)
const line = lines[offset]
+ if (!line) {
+ return false
+ }
- const isUserLand = !line?.includes('node_modules') && !line?.includes('node:internal')
+ const isUserLand = !line.includes('node_modules') && !line.includes('node:internal')
return isUserLand
}📝 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.
| function isUserLandCode(offset: number = 1): boolean { | |
| const stack = getStackTraceLines() | |
| const lines = stack.slice(2) | |
| const line = lines[offset] | |
| const isUserLand = !line?.includes('node_modules') && !line?.includes('node:internal') | |
| return isUserLand | |
| } | |
| function isUserLandCode(offset: number = 1): boolean { | |
| const stack = getStackTraceLines() | |
| const lines = stack.slice(2) | |
| const line = lines[offset] | |
| if (!line) { | |
| return false | |
| } | |
| const isUserLand = !line.includes('node_modules') && !line.includes('node:internal') | |
| return isUserLand | |
| } |
🤖 Prompt for AI Agents
In `@src/runtime/prerender/plugin.server.ts` around lines 38 - 44, The function
isUserLandCode misclassifies an undefined stack line as user-land; modify
isUserLandCode so it first checks that the selected line exists (from
getStackTraceLines and the local variable line) and returns false if line is
undefined or empty, then only evaluate !line.includes('node_modules') &&
!line.includes('node:internal') to determine user-land; locate function
isUserLandCode and add an explicit existence check for line before applying
includes.
| /** | ||
| * Get all stacktrace lines without the current file | ||
| * Don't use in build files if we ever goes into build mode nuxt hiints. Thank you. | ||
| */ |
There was a problem hiding this comment.
Typo in comment.
"hiints" should be "hints".
/**
* Get all stacktrace lines without the current file
- * Don't use in build files if we ever goes into build mode nuxt hiints. Thank you.
+ * Don't use in build files if we ever go into build mode nuxt hints. Thank you.
*/📝 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.
| /** | |
| * Get all stacktrace lines without the current file | |
| * Don't use in build files if we ever goes into build mode nuxt hiints. Thank you. | |
| */ | |
| /** | |
| * Get all stacktrace lines without the current file | |
| * Don't use in build files if we ever go into build mode nuxt hints. Thank you. | |
| */ |
🤖 Prompt for AI Agents
In `@src/runtime/prerender/utils.ts` around lines 1 - 4, Fix the typo in the top
comment block that begins with "Get all stacktrace lines without the current
file" by replacing "hiints" with "hints" so the comment reads correctly; update
that docstring/comment text in the utils file (the block containing "Don't use
in build files if we ever goes into build mode nuxt hiints. Thank you.") to use
"hints".
|
|
||
| const ssrContext = nuxtApp.ssrContext | ||
| // Access to any property on ssrContext will mark the page as non-prerenderable | ||
| Object.defineProperty(nuxtApp, 'ssrContext', { |
There was a problem hiding this comment.
I think it depends what you're accessing. ssrContext.event.url would be fine, for example.
There was a problem hiding this comment.
Is there any other property you think it is fine ? or should we check request headers and event.context acess only ?
🔗 Linked issue
resolve #80
❓ Type of change
📚 Description
This PR adds prerender hints based on ssrContext properties access. For now, it's better to not set any filters to avoid false positive.
TODO
useEvent/nuxtApp.ssrContextcomposables to set the value to falseNext step: