-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat(nuxt): support lazy hydration macros #31192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(nuxt): support lazy hydration macros #31192
Conversation
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #31192 will not alter performanceComparing Summary
|
6af4e1c
to
a0a16b0
Compare
a0a16b0
to
7710482
Compare
WalkthroughThis update introduces a macro-based system for defining lazy hydration strategies in Nuxt applications. A new macro, 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (10)
docs/3.api/3.utils/define-lazy-media-query-component.md (1)
50-50
: Add parentheses around the media query value for consistency.The media query value doesn't include parentheses in this example, but the prop name and earlier example suggest they should be included.
- <LazyMediaQueryMyComponent hydrate-on-media-query="min-width: 768px" /> + <LazyMediaQueryMyComponent hydrate-on-media-query="(min-width: 768px)" />docs/3.api/3.utils/define-lazy-never-component.md (1)
34-34
: Consider adding missing documentation sections.Unlike the other lazy hydration component documentation, this file doesn't include an Emits section. For consistency, consider adding an empty emits section or clarify if this component doesn't emit events.
If the component doesn't emit any events, consider adding:
## Emits This component doesn't emit any events since it's never hydrated.
docs/3.api/3.utils/define-lazy-idle-component.md (1)
46-46
: Clarify the default timeout duration.The documentation mentions a "default timeout duration" when passing
true
, but doesn't specify what that duration is.Consider updating to include the actual default timeout value:
-You can pass a number to specify a timeout duration. If hydration has not been triggered within this duration, it will occur automatically. Alternatively, pass `true` to use the default timeout duration. The number must be a positive value; otherwise, it will be ignored. +You can pass a number to specify a timeout duration in milliseconds. If hydration has not been triggered within this duration, it will occur automatically. Alternatively, pass `true` to use the default timeout duration (typically 5000ms, but may vary). The number must be a positive value; otherwise, it will be ignored.docs/3.api/3.utils/define-lazy-interaction-component.md (1)
15-17
: Fix formatting issue with tip block.There appears to be a formatting issue with the tip block delimiter where it's being detected as "loose punctuation".
Add a new line before the
::tip
directive to ensure proper Markdown rendering:- When using `v-if="false"` on a lazy component, you might not need delayed hydration. You can just use a normal lazy component. - ::tip + When using `v-if="false"` on a lazy component, you might not need delayed hydration. You can just use a normal lazy component. + + ::tip🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...until the component is actually needed. :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/model-event.vue (1)
13-16
: Consider enhancing the logging function with more detailed information.The current logging function is simple and functional but could be more informative for debugging purposes.
Consider enhancing the log function to include more details, like a timestamp:
-function log () { - console.log('Component hydrated') +function log () { + console.log(`Component hydrated at ${new Date().toISOString()}`)docs/3.api/3.utils/define-lazy-if-component.md (1)
15-17
: Fix formatting issue with tip block.There appears to be a formatting issue with the tip block delimiter where it's being detected as "loose punctuation".
Add a new line before the
::tip
directive to ensure proper Markdown rendering:- When using `v-if="false"` on a lazy component, you might not need delayed hydration. You can just use a normal lazy component. - ::tip + When using `v-if="false"` on a lazy component, you might not need delayed hydration. You can just use a normal lazy component. + + ::tip🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...ng resources are only used when needed. :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
packages/nuxt/test/lazy-hydration-macro-transform.test.ts (2)
80-81
: Consider consolidating repeated checks.Each test suite repeats the same transformation check and similar assertions. Extracting these repeated snippet checks into a shared helper function could remove duplication and make the tests more maintainable.
134-185
: Expand testing for corner cases.The
transform
function currently tests positive scenarios of matching macros. Adding tests for corner cases (e.g., partial macro matches, malformed imports, or multiple macros in a single line) can help ensure robustness.Would you like assistance creating additional test cases that validate failure modes or handle advanced edge cases?
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (2)
18-19
: Watch out for potential edge cases.Relying on these regex patterns for matching may capture unexpected code fragments if a file inadvertently contains similar syntax. Consider adding stricter checks or fallback warnings to notify developers of potential partial matches.
53-62
: Add a diagnostic log or warning.When a variable name or component name is missing, the code is simply removed. Logging a small warning would help developers catch unintentional misconfigurations or typos.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/3.api/3.utils/define-lazy-after-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-idle-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-if-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-interaction-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-media-query-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-never-component.md
(1 hunks)docs/3.api/3.utils/define-lazy-visible-component.md
(1 hunks)packages/nuxt/src/components/module.ts
(2 hunks)packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
(1 hunks)packages/nuxt/src/components/runtime/lazy-hydrated-component.ts
(3 hunks)packages/nuxt/test/lazy-hydration-macro-transform.test.ts
(1 hunks)test/basic.test.ts
(1 hunks)test/fixtures/basic/pages/lazy-import-components/delayed-hydration/index.vue
(1 hunks)test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/index.vue
(1 hunks)test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/model-event.vue
(1 hunks)test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/time.vue
(1 hunks)test/fixtures/basic/pages/lazy-import-components/index.vue
(0 hunks)
💤 Files with no reviewable changes (1)
- test/fixtures/basic/pages/lazy-import-components/index.vue
🧰 Additional context used
🪛 LanguageTool
docs/3.api/3.utils/define-lazy-interaction-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyInteractionC...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...until the component is actually needed. :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-if-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use defineLazyIfComponent
...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...ng resources are only used when needed. :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-visible-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyVisibleCompo...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...egy to ensure a better user experience. :: ::note Under the hood, this uses Vue'...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ...to ensure a better user experience. :: ::note Under the hood, this uses Vue's bu...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...ponents/async.html#hydrate-on-visible). :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...'100px' }" /> ``` ::read-more{to="https://developer.mozilla...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~61-~61: Loose punctuation mark.
Context: ...t the options for hydrate-on-visible
. :: ## Emits ### hydrated
- Type:...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-after-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyAfterCompone...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ... competition for main thread resources. :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-idle-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyIdleComponen...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ... competition for main thread resources. :: ::note Under the hood, this uses Vue'...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ...petition for main thread resources. :: ::note Under the hood, this uses Vue's bu...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...components/async.html#hydrate-on-idle). :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-media-query-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyMediaQueryCo...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...g hydration only occurs when necessary. :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-never-component.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::note This component will never be hydra...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ... it should not be set to never hydrate. :: ## Usage ```vue <script setup lang="...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 GitHub Actions: docs
docs/3.api/3.utils/define-lazy-interaction-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-if-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-visible-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-after-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-idle-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-media-query-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
docs/3.api/3.utils/define-lazy-never-component.md
[error] 1-1: [404] Network error: Not Found for https://github.com/nuxt/nuxt/blob/main/packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
🔇 Additional comments (38)
docs/3.api/3.utils/define-lazy-media-query-component.md (2)
11-11
: Clear and helpful explanation of the macro purpose.The documentation provides a clear explanation of the purpose and benefits of using this macro.
15-17
: Good usage tip included.The tip provides valuable advice on when to use this component, focusing on optimization for specific screen sizes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...g hydration only occurs when necessary. :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
docs/3.api/3.utils/define-lazy-never-component.md (1)
15-17
: Excellent cautionary note about use cases.The note provides crucial information about when this component should and should not be used, which helps prevent misuse.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ... it should not be set to never hydrate. :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
packages/nuxt/src/components/module.ts (2)
18-18
: Clean import of new lazy hydration functionality.The import is appropriately added alongside other related imports.
226-232
: Well-structured conditional plugin registration.The code properly adds the new macro transform plugin and type template only when the experimental flag is enabled, following the same pattern as the existing lazy hydration functionality.
The implementation appropriately:
- Uses the same shared loader options as other plugins
- Handles sourcemap configuration consistently
- Adds both the transform plugin and type template
docs/3.api/3.utils/define-lazy-idle-component.md (1)
19-21
: Good reference to Vue's underlying implementation.The note helpfully links to Vue's official documentation, providing context about the underlying implementation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...components/async.html#hydrate-on-idle). :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/time.vue (3)
1-8
: LGTM! Template structure is well organised.The template follows good practices for Vue components with clear element nesting and props assignment. The
data-testid
attribute will be useful for testing the hydration functionality.
10-14
: Component name is well-structured but there's a warning comment that should be addressed.The warning comment in line 11 indicates an issue with using reserved HTML elements as component id. Although the component's name is appropriate here (LazyTimeDelayedComponentMacroPage), the warning suggests there might be an internal naming conflict with "time".
Please check if this warning is expected or if adjustments are needed to prevent conflicts with the HTML "time" element.
16-17
: Implementation is correct.The lazy time component is correctly defined using the
defineLazyTimeComponent
macro, which properly imports the delayed component.docs/3.api/3.utils/define-lazy-interaction-component.md (4)
11-14
: Documentation introduction is clear and concise.The introduction effectively explains the purpose of the macro and provides important context for its usage.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyInteractionC...(UNLIKELY_OPENING_PUNCTUATION)
19-33
: Usage example is clear and relevant.The example effectively demonstrates how to use the macro and apply the hydration strategy with appropriate attribute binding.
35-54
: Props documentation is complete and well-structured.The documentation for the
hydrateOnInteraction
prop is thorough, including its type, default value, and usage example.
56-63
: Emits documentation is concise and clear.The
hydrated
event is properly documented with its type and description.test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/model-event.vue (2)
1-8
: LGTM! Template structure effectively demonstrates v-model binding and event handling.The template clearly shows how to use the lazy interaction component with v-model binding and how to listen for the hydrated event.
10-12
: Component definition is implemented correctly.The lazy interaction component is properly defined using the macro. This demonstrates the correct pattern for component importation and preparation for lazy hydration.
docs/3.api/3.utils/define-lazy-if-component.md (4)
11-14
: Documentation introduction is clear and informative.The introduction effectively explains the purpose of the macro and its practical use case.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can usedefineLazyIfComponent
...(UNLIKELY_OPENING_PUNCTUATION)
19-40
: Usage example is comprehensive and practical.The example effectively demonstrates the workflow of defining a component that will be hydrated conditionally, including setting up the reactive condition. This provides a complete picture of how the macro should be used.
42-58
: Props documentation is complete and includes helpful examples.The documentation for the
hydrateWhen
prop is thorough, including its type, default value, and a clear example showing practical usage.
60-66
: Emits documentation is clear and consistent.The
hydrated
event is properly documented with its type and description, matching the pattern used in other similar documentation.test/fixtures/basic/pages/lazy-import-components/delayed-hydration/index.vue (2)
1-55
: Comprehensive test file covering multiple lazy hydration strategiesThis new test fixture effectively demonstrates all the different lazy hydration strategies introduced in this PR. The file includes components with various hydration approaches:
- Interaction-based hydration with both default and customised event triggers
- Visibility-based hydration (both at the top and bottom of the page)
- Condition-based hydration using state
- Idle-based hydration
- Never hydration
The tall div helps validate the visibility-based hydration by forcing scroll, which is a clever testing approach.
58-60
: Appropriate state management implementationThe implementation uses
useState
for managing the condition state, which is the recommended approach in Nuxt. The initial value is properly set to 1, and the state name provides clear context about its purpose.packages/nuxt/src/components/runtime/lazy-hydrated-component.ts (3)
32-36
: Improved developer experience with sensible defaultsAdding a default value of
true
for thehydrateOnVisible
property makes the API more user-friendly. This change allows developers to use the component with less boilerplate by simply adding thehydrate-on-visible
attribute without values.
43-47
: Consistent API improvements for idle hydrationMaking the
hydrateOnIdle
property non-required with a default value oftrue
maintains consistency with other hydration strategies and improves the developer experience. This change aligns with the pattern established for the visible hydration property.
110-114
: Enhanced developer experience for never-hydrated componentsMaking the
hydrateNever
property non-required with a default value oftrue
simplifies usage of never-hydrated components. This change follows the same pattern as the other hydration strategies, creating a consistent API surface across all lazy hydration components.docs/3.api/3.utils/define-lazy-visible-component.md (4)
11-22
: Clear explanation of the lazy visible component conceptThe introduction effectively explains the purpose and behaviour of the
defineLazyVisibleComponent
macro. The tip and note sections provide valuable guidance on when to use this feature and how it works under the hood, which helps developers make informed decisions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyVisibleCompo...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...egy to ensure a better user experience. :: ::note Under the hood, this uses Vue'...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~18-~18: Loose punctuation mark.
Context: ...to ensure a better user experience. :: ::note Under the hood, this uses Vue's bu...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...ponents/async.html#hydrate-on-visible). :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
23-38
: Straightforward usage exampleThe example demonstrates a clear implementation pattern that's easy to follow. It shows both the component definition in the script section and its usage in the template.
39-62
: Comprehensive props documentation with examplesThe documentation for the
hydrateOnVisible
prop is thorough, including its type, default value, and a practical example demonstrating how to configure the Intersection Observer options. The link to MDN for further reading is a helpful addition.🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...'100px' }" /> ``` ::read-more{to="https://developer.mozilla...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~61-~61: Loose punctuation mark.
Context: ...t the options forhydrate-on-visible
. :: ## Emits ###hydrated
- Type:...(UNLIKELY_OPENING_PUNCTUATION)
64-71
: Complete event documentationThe documentation clearly describes the
hydrated
event, which is important for developers who need to respond to component hydration completion.docs/3.api/3.utils/define-lazy-after-component.md (4)
11-18
: Clear explanation with helpful usage tipThe documentation effectively explains the purpose of
defineLazyAfterComponent
and includes a useful tip about when to use it versus a normal lazy component. This guidance helps developers make informed decisions about component hydration strategies.🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ... can just use a normal lazy component. ::tip You can use `defineLazyAfterCompone...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ... competition for main thread resources. :: ## Usage ```vue <script setup lang="...(UNLIKELY_OPENING_PUNCTUATION)
19-34
: Clear usage exampleThe example clearly demonstrates how to define and use a time-delayed component, making it easy for developers to implement in their own applications.
35-51
: Well-documented props with examplesThe documentation for the
hydrateAfter
prop is thorough, including its type and a practical example showing how to set the delay value. This makes it easy for developers to understand and implement the functionality.
52-59
: Complete event documentationThe documentation clearly describes the
hydrated
event, which is important for developers who need to respond to component hydration completion.packages/nuxt/test/lazy-hydration-macro-transform.test.ts (1)
36-38
: Good coverage of transformed imports.Verifying that the transformed code contains the expected import statements is an effective way to ensure correctness. This approach accurately confirms that macros are transformed into their corresponding lazy hydration component creators.
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (1)
64-69
: Give developers feedback when no matching component is found.Currently, the macro call is removed from the source if no component is located. Providing a compile-time warning or error message can help users detect and correct incorrect or missing component configurations faster.
test/fixtures/basic/pages/lazy-import-components/delayed-hydration/macro/index.vue (2)
15-17
: Validate parentheses in the media query attribute.Ensure that the
(max-width: 768px)
syntax is interpreted correctly by all targeted browsers. Confirm that the media query is valid and triggers as intended for different screen sizes.
25-34
: Great demonstration of reactive condition usage.Using a reactive state to drive conditional hydration showcases the macro’s flexibility. Consider adding a quick test verifying that the component remains unhydrated when
state <= 0
to ensure the lazy condition works reliably.test/basic.test.ts (2)
2871-2984
: Well-structured test suite for delayed hydration components.This new test suite effectively validates the behavior of delayed hydration components, covering essential scenarios:
- Visibility-based hydration
- Interaction-triggered hydration
- Time-based hydration
- Conditional hydration
- Component reactivity
- Event emission
The tests validate both the server-rendered state (unhydrated) and client-side behavior after hydration triggers, ensuring the functionality works as expected across the application lifecycle.
2986-3090
: Comprehensive test coverage for Vue macro implementations of lazy hydration.This parallel test suite ensures that the macro-based implementation (
defineLazyVisibleComponent
,defineLazyIdleComponent
, etc.) behaves identically to the standard implementation. The tests thoroughly verify:
- Components hydrate under the correct conditions
- Custom triggers override default behavior
- Time-based hydration works properly
- Components maintain reactivity with model bindings
- Hydration events are emitted correctly
Maintaining separate test suites for both implementations ensures consistent behavior across different usage approaches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's important to align with the hydration props, for clarity.
We could use: defineHydrateAfterComponent
, defineHydrateOnIdleComponent
, defineHydrateWhenComponent
, defineHydrateOnInteractionComponent
, defineHydrateOnMediaQueryComponent
, defineHydrateNeverComponent
, defineHydrateOnVisibleComponent
,
Let's use a single defineLazyHydrationComponent(hydrationStrategy, () => import(''))
for better DX - I think it would be just as doable to use as a macro. And it simplifies the documentation too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (6)
18-18
: Consider splitting the complex regex for better maintainabilityThis regex is quite complex and might be difficult to maintain or debug. Consider breaking it down into smaller, named parts or adding comments explaining the different capturing groups.
-const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/g +// Match parts of the lazy hydration macro: +// 1. Optional variable declaration (const|let|var name =) +// 2. Function name (defineLazyHydrationComponent) +// 3. Hydration strategy string ('visible', 'idle', etc.) +// 4. Dynamic import path +const VARIABLE_DECLARATION_RE = /\b(?:const|let|var)\s+(\w+)\s*=\s*/ +const HYDRATION_STRATEGY_RE = /['"]([^'"]+)['"]/ +const DYNAMIC_IMPORT_RE = /\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)/ +const LAZY_HYDRATION_MACRO_RE = new RegExp( + `(?:${VARIABLE_DECLARATION_RE.source})?defineLazyHydrationComponent\\(\\s*${HYDRATION_STRATEGY_RE.source}\\s*,\\s*${DYNAMIC_IMPORT_RE.source}\\s*\\)`, + 'g' +)
59-62
: Add warning message for missing variable nameWhen validation fails, the code silently removes the macro. Adding warning messages would help developers understand why their code is being removed.
if (!variableName) { + console.warn(`[nuxt] Lazy hydration macro at position ${startIndex} has no variable name and will be ignored.`) s.remove(startIndex, endIndex) continue }
64-67
: Add specific error message for invalid hydration strategyProviding a specific error message listing the valid strategies would be more helpful than silently removing the code.
if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) { + console.warn( + `[nuxt] Invalid hydration strategy '${hydrationStrategy}' at position ${startIndex}. ` + + `Valid strategies are: ${HYDRATION_STRATEGY.join(', ')}` + ) s.remove(startIndex, endIndex) continue }
75-80
: Optimize component lookup for better performanceThe current implementation searches through the entire components array for each match. For better performance with many components, consider creating a lookup map outside the loop.
const components = options.getComponents() +const componentMap = new Map( + components.map(c => [c.pascalName, c]) +) for (const match of matches) { // ... const name = componentNameMatch[1] - const component = findComponent(components, name) + const pascalName = pascalCase(name) + const component = componentMap.get(pascalName) if (!component) { s.remove(startIndex, endIndex) continue } } // Remove or mark as deprecated the now-unused findComponent function
82-86
: Add error handling for dynamic importsThe generated code doesn't include error handling for the dynamic import. Consider generating code that catches import errors to provide a better developer experience.
const relativePath = relative(options.srcDir, component.filePath) -const dynamicImport = `${genDynamicImport(component.filePath, { interopDefault: false })}.then(c => c.${component.export ?? 'default'} || c)` +const dynamicImport = `${genDynamicImport(component.filePath, { interopDefault: false })} + .then(c => c.${component.export ?? 'default'} || c) + .catch(err => { + console.error(\`Failed to load lazy component from ${relativePath}: \${err}\`) + return {} + })` const replaceFunctionName = `createLazy${upperFirst(hydrationStrategy)}Component` const replacement = `const ${variableName} = __${replaceFunctionName}(${JSON.stringify(relativePath)}, ${dynamicImport})`
109-112
: Consider inline documentation for the helper functionAdd JSDoc comments to the helper function to explain its purpose and parameters.
+/** + * Find a component by its name in the components array + * + * @param components - Array of components to search + * @param name - The component name to find (will be converted to PascalCase) + * @returns The found component or undefined + */ function findComponent (components: Component[], name: string) { const id = pascalCase(name) return components.find(c => c.pascalName === id) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (2)
packages/schema/src/index.ts (2)
Component
(3-3)ComponentsOptions
(3-3)test/fixtures/basic/nuxt.config.ts (1)
components
(237-243)
🔇 Additional comments (5)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (5)
1-8
: LGTM - Import statements look goodThe imports are well organised and appropriately chosen for the task. Using platform-agnostic utilities like
pathe
and specialised libraries likeknitwork
andscule
is a good practice.
10-16
: LGTM - Interface definition is clear and well-typedThe
LoaderOptions
interface provides a clear contract for what the plugin needs to function, with appropriate types and optional flags.
29-37
: LGTM - File filtering logic is robustThe transformation inclusion logic properly respects explicit include/exclude patterns and falls back to file type detection for Vue templates/scripts and JS files.
91-94
: Clever approach for avoiding import conflictsPrefixing imported function names with
__
and usinggenImport
is a smart way to avoid conflicts with user code. This ensures that the generated code won't clash with any existing imports of the same functions.
96-103
: LGTM - Proper sourcemap handlingThe code correctly generates source maps when enabled, which will help developers debug their code when using this transformation.
…ydration-micro # Conflicts: # packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
@Mini-ghost |
@jakubm95 If everything goes smoothly, I believe it might be included in the 3.18 release. |
…ydration-micro # Conflicts: # packages/nuxt/src/components/module.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/basic.test.ts (1)
2893-2893
: Consider reducing test timeout.The 20-second timeout seems excessive for these hydration tests and may slow down the test suite unnecessarily.
- it('lazy load delayed hydration comps at the right time', { timeout: 20_000 }, async () => { + it('lazy load delayed hydration comps at the right time', { timeout: 10_000 }, async () => {Most hydration operations should complete well within 10 seconds, and this would improve test suite performance.
Also applies to: 3008-3008
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/nuxt/src/components/module.ts
(3 hunks)packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
(4 hunks)packages/nuxt/src/imports/presets.ts
(1 hunks)test/basic.test.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxt/src/imports/presets.ts
- packages/nuxt/src/components/module.ts
🧰 Additional context used
🪛 GitHub Check: build
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
[failure] 6-6:
Module '"../../core/utils/parse"' has no exported member 'parseAndWalk'.
[failure] 6-6:
Module '"../../core/utils/parse"' has no exported member 'ScopeTracker'.
🪛 GitHub Check: code
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
[failure] 51-51: packages/nuxt/test/lazy-hydration-macro-transform.test.ts > lazy hydration macro transform > should correctly transform lazy hydration macro in sfc
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-never
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-when
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-after
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-media-query
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-interaction
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-visible
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-idle
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve components
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
🪛 GitHub Actions: ci
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
[error] 6-6: TypeScript error TS2305: Module '../../core/utils/parse' has no exported member 'ScopeTracker'.
🔇 Additional comments (4)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (3)
50-51
: Update handler signature and scope tracker initialisation once imports are fixed.The handler signature change to include
id
parameter and scope tracker initialisation look correct, but depend on the import fix.🧰 Tools
🪛 GitHub Check: code
[failure] 51-51: packages/nuxt/test/lazy-hydration-macro-transform.test.ts > lazy hydration macro transform > should correctly transform lazy hydration macro in sfc
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-never
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-when
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-after
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-media-query
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-interaction
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-visible
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components hydrate-on-idle
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve lazy hydration components
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
[failure] 51-51: packages/nuxt/test/component-loader.test.ts > components:loader > should correctly resolve components
TypeError: ScopeTracker is not a constructor
❯ Object.handler packages/nuxt/src/components/plugins/lazy-hydration-transform.ts:51:30
❯ Object. node_modules/.pnpm/unplugin@2.3.5/node_modules/unplugin/dist/index.js:1512:19
❯ node_modules/.pnpm/rollup@4.41.1/node_modules/rollup/dist/es/shared/node-entry.js:22249:40⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'PLUGIN_ERROR', plugin: 'nuxt:components-loader-pre', hook: 'transform', id: '/pages/index.vue' }
53-60
: Robust script parsing with error handling.The script parsing loop correctly handles potential parsing errors with try-catch blocks whilst building the scope tracker for each script block.
79-81
: Scope-aware component filtering prevents transformation conflicts.The scope checking logic correctly prevents transformation of locally declared components, addressing the auto-import conflict mentioned in the PR objectives.
test/basic.test.ts (1)
2892-3005
: Well-structured test suite for delayed hydration components.The test coverage is comprehensive, testing all major hydration strategies (visible, interaction, time, never) with proper async handling and cleanup. The tests appropriately verify both server-side rendering (unhydrated state) and client-side behavior (hydrated state).
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
Outdated
Show resolved
Hide resolved
@@ -13,6 +14,7 @@ | |||
transform?: ComponentsOptions['transform'] | |||
} | |||
|
|||
const SCRIPT_RE = /(?<=<script[^>]*>)[\s\S]*?(?=<\/script>)/gi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the updated SCRIPT_RE pattern handles all edge cases.
The regex pattern has been updated to use lookahead/lookbehind assertions. Ensure this correctly handles various script tag formats whilst maintaining performance.
Test the regex pattern against various script tag formats:
🏁 Script executed:
#!/bin/bash
# Description: Test the SCRIPT_RE pattern against various script tag formats
# Create test cases for different script tag formats
cat > test_script_tags.html << 'EOF'
<script>content1</script>
<SCRIPT>content2</SCRIPT>
<script type="text/javascript">content3</script>
<script lang="ts">content4</script>
<script setup>content5</script>
<script > content6 </script >
<script >content7</script >
EOF
# Test with a simple Node.js script
node -e "
const content = require('fs').readFileSync('test_script_tags.html', 'utf8');
const SCRIPT_RE = /(?<=<script[^>]*>)[\s\S]*?(?=<\/script>)/gi;
const matches = [...content.matchAll(SCRIPT_RE)];
console.log('Found', matches.length, 'matches:');
matches.forEach((match, i) => console.log(i+1, ':', match[0].trim()));
"
rm test_script_tags.html
Length of output: 476
Update SCRIPT_RE to handle optional whitespace in closing tags
The current regex fails to match <script >…</script >
and <script\t>…</script\t>
because the lookahead only matches </script>
with no trailing whitespace. Update both opening and closing tag patterns to allow optional whitespace and a word boundary:
Locations:
- packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (around line 17)
Suggested diff:
- const SCRIPT_RE = /(?<=<script[^>]*>)[\s\S]*?(?=<\/script>)/gi
+ const SCRIPT_RE = /(?<=<script\b[^>]*>)[\s\S]*?(?=<\/script\b[^>]*>)/gi
With this change the regex correctly matches all of:
<script>…</script>
<SCRIPT>…</SCRIPT>
<script type="text/javascript">…</script>
<script lang="ts">…</script>
<script setup>…</script>
<script > … </script >
<script >…</script >
📝 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 SCRIPT_RE = /(?<=<script[^>]*>)[\s\S]*?(?=<\/script>)/gi | |
const SCRIPT_RE = /(?<=<script\b[^>]*>)[\s\S]*?(?=<\/script\b[^>]*>)/gi |
🤖 Prompt for AI Agents
In packages/nuxt/src/components/plugins/lazy-hydration-transform.ts at line 17,
the SCRIPT_RE regex does not handle optional whitespace in closing script tags,
causing it to miss matches like <script >…</script > and <script\t>…</script\t>.
Update the regex to allow optional whitespace and a word boundary in both the
opening and closing script tag patterns, ensuring it matches all script tag
variations including those with spaces or tabs before the closing angle bracket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (3)
18-18
: Consider improving regex maintainability and robustness.The regex pattern is quite complex and may be fragile to syntax variations. Consider the following improvements:
- Whitespace handling: The current regex might not handle all valid JavaScript whitespace patterns
- Quote variations: Mixed single/double quotes in the same expression might not be handled correctly
- Maintainability: The complex regex could be split into smaller, more readable parts
Consider refactoring to use AST parsing for more robust macro detection:
-const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/g +// Consider using AST parsing with @babel/parser or acorn for more robust detection +const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\s*\(\s*['"`]([^'"`]+)['"`]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"`]([^'"`]+)['"`]\s*\)\s*\)/g
82-88
: Robust transformation logic with minor suggestions.The dynamic import generation and replacement logic is well-implemented. The handling of relative paths, interop defaults, and function naming follows good patterns.
Consider adding validation for edge cases:
const relativePath = relative(options.srcDir, component.filePath) + if (!relativePath) { + console.warn(`[nuxt:lazy-hydration-macro] Could not determine relative path for component '${name}'`) + s.remove(startIndex, endIndex) + continue + } const dynamicImport = `${genDynamicImport(component.filePath, { interopDefault: false })}.then(c => c.${component.export ?? 'default'} || c)`
22-107
: Well-architected plugin with room for enhanced developer experience.The plugin implementation successfully addresses the PR objectives and follows Nuxt plugin patterns correctly. The use of unplugin ensures cross-bundler compatibility, and the transformation logic properly handles the macro-to-function conversion.
Strengths:
- Efficient file filtering and code transformation
- Proper handling of import conflicts with prefixed names
- Good integration with Nuxt's component system
- Comprehensive hydration strategy support
Areas for enhancement:
- Error messaging for invalid macro usage (as noted in previous comments)
- Regex robustness for edge cases in JavaScript syntax
Would you like me to help generate more comprehensive error handling or create tests for edge cases in the regex matching?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nuxt/src/components/module.ts
(3 hunks)packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/components/module.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (2)
packages/nuxt/src/core/utils/plugins.ts (1)
isJS
(40-44)test/fixtures/basic/nuxt.config.ts (1)
components
(234-240)
🔇 Additional comments (3)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (3)
1-16
: Well-structured imports and interface definition.The imports are appropriate for the plugin's functionality, and the
LoaderOptions
interface clearly defines all necessary configuration properties.
91-103
: Excellent implementation of conflict-free import generation.The use of prefixed imports (
__${name}
) effectively addresses the potential naming conflicts mentioned in previous reviews. The conditional import generation and sourcemap handling are well-implemented.
109-112
: Simple and effective component lookup helper.The helper function correctly handles PascalCase conversion and component matching. The implementation is clean and efficient.
if (!variableName) { | ||
s.remove(startIndex, endIndex) | ||
continue | ||
} | ||
|
||
if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) { | ||
s.remove(startIndex, endIndex) | ||
continue | ||
} | ||
|
||
const componentNameMatch = matchedString.match(COMPONENT_NAME) | ||
if (!componentNameMatch || !componentNameMatch[1]) { | ||
s.remove(startIndex, endIndex) | ||
continue | ||
} | ||
|
||
const name = componentNameMatch[1] | ||
const component = findComponent(components, name) | ||
if (!component) { | ||
s.remove(startIndex, endIndex) | ||
continue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling for better developer experience.
The current implementation silently removes invalid macro calls, which could confuse developers who expect their code to work but see it removed without explanation.
Consider adding warning messages for invalid macro usage:
if (!variableName) {
+ console.warn(`[nuxt:lazy-hydration-macro] Invalid macro usage: variable name is required`)
s.remove(startIndex, endIndex)
continue
}
if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) {
+ console.warn(`[nuxt:lazy-hydration-macro] Invalid hydration strategy: ${hydrationStrategy}. Valid strategies: ${HYDRATION_STRATEGY.join(', ')}`)
s.remove(startIndex, endIndex)
continue
}
const componentNameMatch = matchedString.match(COMPONENT_NAME)
if (!componentNameMatch || !componentNameMatch[1]) {
+ console.warn(`[nuxt:lazy-hydration-macro] Could not extract component name from import`)
s.remove(startIndex, endIndex)
continue
}
const name = componentNameMatch[1]
const component = findComponent(components, name)
if (!component) {
+ console.warn(`[nuxt:lazy-hydration-macro] Component '${name}' not found in component registry`)
s.remove(startIndex, endIndex)
continue
}
📝 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.
if (!variableName) { | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) { | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
const componentNameMatch = matchedString.match(COMPONENT_NAME) | |
if (!componentNameMatch || !componentNameMatch[1]) { | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
const name = componentNameMatch[1] | |
const component = findComponent(components, name) | |
if (!component) { | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
if (!variableName) { | |
console.warn(`[nuxt:lazy-hydration-macro] Invalid macro usage: variable name is required`) | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) { | |
console.warn( | |
`[nuxt:lazy-hydration-macro] Invalid hydration strategy: ${hydrationStrategy}. ` + | |
`Valid strategies: ${HYDRATION_STRATEGY.join(', ')}` | |
) | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
const componentNameMatch = matchedString.match(COMPONENT_NAME) | |
if (!componentNameMatch || !componentNameMatch[1]) { | |
console.warn(`[nuxt:lazy-hydration-macro] Could not extract component name from import`) | |
s.remove(startIndex, endIndex) | |
continue | |
} | |
const name = componentNameMatch[1] | |
const component = findComponent(components, name) | |
if (!component) { | |
console.warn( | |
`[nuxt:lazy-hydration-macro] Component '${name}' not found in component registry` | |
) | |
s.remove(startIndex, endIndex) | |
continue | |
} |
🤖 Prompt for AI Agents
In packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
between lines 59 and 80, the code silently removes invalid macro calls without
notifying the developer. To improve developer experience, add warning messages
using console.warn or a logger before each s.remove call to indicate why the
macro call is being removed, specifying the exact validation failure such as
missing variableName, invalid hydrationStrategy, missing componentNameMatch, or
component not found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (1)
59-80
: Improve error handling for better developer experience.The current implementation silently removes invalid macro calls, which could confuse developers who expect their code to work but see it removed without explanation.
Consider adding warning messages for invalid macro usage as suggested in previous reviews, using
console.warn
or a logger before eachs.remove
call to indicate why the macro call is being removed.
🧹 Nitpick comments (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (1)
18-20
: Consider improving regex robustness for macro detection.The current regex pattern is quite strict and may not handle all valid JavaScript/TypeScript syntax variations (e.g., different spacing, comments, or template literals). Consider using an AST-based approach for more reliable parsing, especially given the complexity of the macro pattern.
Example issues the current regex might miss:
- Extra whitespace or comments between tokens
- Template literals instead of string literals
- Different variable declaration styles
-const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/g +// Consider using AST-based parsing for more robust detection +// This could use @babel/parser or similar to handle edge cases
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
(1 hunks)packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
(3 hunks)packages/nuxt/src/components/plugins/loader.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (2)
packages/nuxt/src/core/utils/plugins.ts (1)
isJS
(40-44)test/fixtures/basic/nuxt.config.ts (1)
components
(234-240)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: lint-docs
- GitHub Check: code
🔇 Additional comments (4)
packages/nuxt/src/components/plugins/loader.ts (1)
8-8
: LGTM! Import reordering for consistency.This stylistic change improves import organisation without affecting functionality.
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (3)
82-94
: Well-designed transformation logic with conflict prevention.The implementation correctly:
- Generates relative paths using the
relative
utility- Creates dynamic imports with proper default fallback handling
- Uses prefixed aliases (
__${replaceFunctionName}
) to prevent naming conflicts- Handles multiple macro calls efficiently
This addresses the import conflict concerns mentioned in previous reviews.
39-43
: Excellent performance optimisation with code filtering.The use of
filter.code.include
with the regex pattern is a smart optimisation that prevents unnecessary processing of files that don't contain lazy hydration macros. This should significantly improve build performance.
109-112
: Simple and effective component lookup utility.The
findComponent
function correctly converts the component name to PascalCase and performs a straightforward lookup. This is appropriately simplified compared to the loader plugin's version since it doesn't need mode-specific handling.
# Conflicts: # test/fixtures/basic/app/pages/lazy-import-components/model-event.vue # test/fixtures/basic/app/pages/lazy-import-components/time.vue # test/fixtures/basic/pages/lazy-import-components/delayed-hydration/model-event.vue # test/fixtures/basic/pages/lazy-import-components/delayed-hydration/time.vue # test/fixtures/basic/pages/lazy-import-components/model-event.vue # test/fixtures/basic/pages/lazy-import-components/time.vue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/basic.test.ts (1)
3009-3113
: The code duplication issue from previous reviews still persists.This test suite continues to duplicate almost all logic from the previous "delayed hydration comps" suite, differing only in endpoint paths with the
/macro
suffix. This violates the DRY principle and creates maintenance overhead as previously identified.
🧹 Nitpick comments (1)
test/fixtures/basic/app/pages/lazy-import-components/macro/time.vue (1)
11-11
: Consider the component naming to avoid HTML element conflicts.The comment indicates a Vue warning about using "time" as a component ID. While the variable name
LazyTimeDelayedComponentMacro
doesn't directly use "time", this warning suggests potential confusion with the HTML<time>
element.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/nuxt/src/imports/presets.ts
(1 hunks)test/basic.test.ts
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/index.vue
(0 hunks)test/fixtures/basic/app/pages/lazy-import-components/macro/index.vue
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/macro/model-event.vue
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/macro/time.vue
(1 hunks)
💤 Files with no reviewable changes (1)
- test/fixtures/basic/app/pages/lazy-import-components/index.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/imports/presets.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: code
- GitHub Check: lint-docs
- GitHub Check: link-checker
🔇 Additional comments (9)
test/fixtures/basic/app/pages/lazy-import-components/macro/time.vue (2)
16-16
: Excellent macro usage demonstration.The
defineLazyHydrationComponent
macro usage correctly demonstrates the time-based hydration strategy with proper TypeScript typing and dynamic import.
3-6
: Proper test setup for time-based hydration.The template correctly demonstrates the time strategy with the
hydrate-after
prop set to 50ms, which is appropriate for testing purposes.test/fixtures/basic/app/pages/lazy-import-components/macro/model-event.vue (3)
11-11
: Correct macro usage for interaction strategy.The
defineLazyHydrationComponent
macro is properly used with the 'interaction' strategy and imports the appropriate component for testing model binding.
3-6
: Excellent demonstration of v-model and event handling.The template correctly shows how lazy hydrated components can handle v-model binding and emit hydration events, which is valuable for testing the macro's integration with Vue's reactivity system.
13-16
: Clean reactive state management.The reactive model and event handler function are properly implemented using the composition API.
test/fixtures/basic/app/pages/lazy-import-components/macro/index.vue (4)
46-51
: Excellent comprehensive macro definitions.All lazy hydration strategies are properly demonstrated with consistent naming patterns and correct macro usage. This provides thorough test coverage for the different hydration strategies.
3-18
: Comprehensive hydration strategy demonstrations.The template effectively showcases multiple hydration strategies with appropriate props:
- Visibility-based hydration with default settings
- Idle hydration
- Interaction hydration with both default and specific event types
- Media query hydration with responsive breakpoint
This provides excellent test coverage for the macro functionality.
19-34
: Effective conditional hydration testing.The implementation demonstrates both static (
hydrate-when="true"
) and reactive (hydrate-when="state > 0"
) conditional hydration, along with user interaction to change the state. This is excellent for testing the 'if' strategy's reactivity.
37-41
: Smart visibility testing setup.The tall div creates the necessary scrolling conditions for testing visibility-based hydration at different viewport positions, which is essential for proper testing of the visible strategy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (1)
59-80
: Improve error handling for better developer experience.The current implementation silently removes invalid macro calls, which could confuse developers. This duplicates a previous review comment but the issue remains unaddressed.
Consider adding warning or error messages for invalid macro usage:
if (!variableName) { + console.warn(`[nuxt:lazy-hydration-macro] Invalid macro usage: variable name is required`) s.remove(startIndex, endIndex) continue } if (!hydrationStrategy || !HYDRATION_STRATEGY.includes(hydrationStrategy)) { + console.warn(`[nuxt:lazy-hydration-macro] Invalid hydration strategy: ${hydrationStrategy}. Valid strategies: ${HYDRATION_STRATEGY.join(', ')}`) s.remove(startIndex, endIndex) continue } const componentNameMatch = matchedString.match(COMPONENT_NAME) if (!componentNameMatch || !componentNameMatch[1]) { + console.warn(`[nuxt:lazy-hydration-macro] Could not extract component name from import`) s.remove(startIndex, endIndex) continue } const name = componentNameMatch[1] const component = findComponent(components, name) if (!component) { + console.warn(`[nuxt:lazy-hydration-macro] Component '${name}' not found in component registry`) s.remove(startIndex, endIndex) continue }
🧹 Nitpick comments (1)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (1)
18-18
: Consider simplifying the complex regex pattern.The current regex pattern is quite complex and may be difficult to maintain. Consider breaking it down into smaller, more readable patterns or using a proper parser for better maintainability.
-const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/g +// Consider breaking down into multiple patterns for better maintainability +const VARIABLE_DECLARATION_RE = /\b(?:const|let|var)\s+(\w+)\s*=\s*/ +const MACRO_CALL_RE = /defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/ +const LAZY_HYDRATION_MACRO_RE = /(?:\b(?:const|let|var)\s+(\w+)\s*=\s*)?defineLazyHydrationComponent\(\s*['"]([^'"]+)['"]\s*,\s*\(\s*\)\s*=>\s*import\s*\(\s*['"]([^'"]+)['"]\s*\)\s*\)/g
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/src/components/module.ts
(3 hunks)packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
(1 hunks)packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxt/src/components/module.ts
- packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: codeql (actions)
- GitHub Check: build
- GitHub Check: lint-docs
- GitHub Check: code
🔇 Additional comments (3)
packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts (3)
92-93
: Consider using import prefixes to avoid conflicts.As mentioned in past comments by danielroe, using import prefixes could help avoid conflicts with existing imports and reduce complexity.
Consider implementing the suggestion to use prefixed imports:
- const imports = genImport(options.clientDelayedComponentRuntime, [...names].map(name => ({ name, as: `__${name}` }))) + const imports = genImport(options.clientDelayedComponentRuntime, [...names].map(name => ({ name, as: `_${name}` })))And update the replacement logic accordingly:
- const replacement = `const ${variableName} = __${replaceFunctionName}(${JSON.stringify(relativePath)}, ${dynamicImport})` + const replacement = `const ${variableName} = _${replaceFunctionName}(${JSON.stringify(relativePath)}, ${dynamicImport})`This aligns with danielroe's suggestion about avoiding the need for complex AST parsing by using prefixed imports.
109-112
: LGTM! Helper function is well-implemented.The
findComponent
function correctly usespascalCase
for name normalisation and finds components by their Pascal case name. The implementation is clean and efficient.
84-85
: ```shell
#!/bin/bashShow the definition of the allowed strategies
rg -n "HYDRATION_STRATEGY" -C 5 packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
Inspect the regex used to capture component name and strategy
rg -n "const COMPONENT_NAME" -C 5 packages/nuxt/src/components/plugins/lazy-hydration-macro-transform.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/time.vue (1)
16-16
: Ensure import path consistency across test fixtures.The import path
'~/components/DelayedComponent.vue'
differs from other files in the same directory that use'~/app/components/DelayedComponent.vue'
. Ensure consistency across all test fixtures.-const LazyTimeDelayedComponentMacro = defineLazyHydrationComponent('time', () => import('~/components/DelayedComponent.vue')) +const LazyTimeDelayedComponentMacro = defineLazyHydrationComponent('time', () => import('~/app/components/DelayedComponent.vue'))test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/model-event.vue (1)
11-11
: Ensure import path consistency across test fixtures.The import path
'~/components/DelayedModel.vue'
should be consistent with other test fixtures using the'~/app/components/'
prefix.-const LazyInteractionDelayedComponentMacro = defineLazyHydrationComponent('interaction', () => import('~/components/DelayedModel.vue')) +const LazyInteractionDelayedComponentMacro = defineLazyHydrationComponent('interaction', () => import('~/app/components/DelayedModel.vue'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/index.vue
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/index.vue
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/model-event.vue
(1 hunks)test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/time.vue
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: codeql (actions)
- GitHub Check: build
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: link-checker
- GitHub Check: code
- GitHub Check: docs
- GitHub Check: lint-docs
🔇 Additional comments (10)
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/time.vue (1)
3-6
: LGTM: Correct time-based hydration configuration.The component correctly demonstrates the 'time' hydration strategy with the
hydrate-after
prop set to 50 milliseconds.test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/model-event.vue (2)
3-6
: LGTM: Excellent demonstration of interaction hydration with model binding.This correctly demonstrates the 'interaction' strategy with v-model binding and hydrated event handling, showcasing the macro's capability for reactive state management.
13-16
: LGTM: Proper reactive state and event handling setup.The reactive model state and hydration event logging function are correctly implemented for testing purposes.
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/index.vue (3)
46-51
: LGTM: Comprehensive demonstration of all hydration strategies.Excellent coverage of all lazy hydration strategies with consistent import paths and proper macro usage. This provides a thorough test fixture for validating the macro functionality.
3-42
: LGTM: Well-structured template demonstrating various hydration triggers.The template effectively demonstrates each hydration strategy with appropriate props and test identifiers, including:
- Visibility-based hydration with scroll testing
- Idle and interaction-based hydration
- Media query and conditional hydration
- Never-hydrate strategy
The structure supports comprehensive testing of all hydration scenarios.
25-34
: LGTM: Proper reactive state management for conditional hydration.The reactive state setup with button interaction correctly demonstrates the 'if' strategy, allowing for dynamic hydration based on changing conditions.
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/index.vue (4)
1-56
: Comprehensive test coverage for lazy hydration strategies.The template effectively demonstrates all major hydration strategies mentioned in the PR:
- Interaction-based hydration (default and specific events)
- Visibility-based hydration (with viewport testing via the tall div)
- Conditional hydration (static and reactive conditions)
- Idle-based hydration with timeout
- Never hydrate strategy
The use of descriptive labels and
data-testid
attributes makes this an excellent test fixture for both manual and automated testing.
58-60
: State management implementation looks correct.The use of
useState
with a descriptive key follows Nuxt conventions properly. The initial value of 1 and the conditionstate > 1
for hydration creates a clear test scenario where the component hydrates after the first button click.
47-49
: Strategic placement of tall div for visibility testing.The 3000px tall div is well-positioned to enable testing of visibility-based hydration by ensuring the bottom
LazyDelayedComponent
is initially out of viewport. This is a practical approach for testing scroll-based visibility triggers.
23-23
: ```shell
#!/bin/bash
set -eecho "Searching for hydrateOnInteraction prop occurrences:"
rg -n "hydrateOnInteraction"echo
echo "Searching for hyphenated prop usage (hydrate-on-interaction):"
rg -n "hydrate-on-interaction"echo
echo "Locating LazyDelayedComponent references:"
rg -n "LazyDelayedComponent"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
</template> | ||
|
||
<script setup lang="ts"> | ||
// [Vue warn]: Do not use built-in or reserved HTML elements as component id: time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address the Vue warning about reserved HTML element names.
The comment indicates a Vue warning about using 'time' as a component ID. This suggests a potential naming conflict with the HTML <time>
element that should be resolved.
Consider renaming the component to avoid conflicts with reserved HTML elements, or investigate if this warning affects functionality.
🤖 Prompt for AI Agents
In
test/fixtures/basic/app/pages/lazy-import-components/delayed-hydration/macro/time.vue
at line 11, the component is named 'time', which conflicts with the built-in
HTML <time> element causing a Vue warning. Rename the component to a
non-reserved, unique name to avoid this conflict and eliminate the warning.
Amazing work - thank you ❤️ |
🔗 Linked issue
📚 Description
This PR introduces a few macros to provide an explicit import solution for lazy hydration.
resolves #31663
TODO
LazyHydrationMacroTransformPlugin
)Visibility Strategy
Visibility strategy enables deferring the hydration of a component until the element(s) become visible in the viewport.
Idle Strategy
Idle strategy enables deferring the hydration of a component until the browser is idle, ensuring that critical rendering tasks are prioritized first.
Interaction Strategy
Interaction strategy enables delaying the hydration of a component until a user interacts with it (e.g., click, mouseover).
Media Query Strategy
Media query strategy enables deferring the hydration of a component until a specified media query matches.
If Strategy
If strategy enables delaying the hydration of a component until a boolean condition is met.
Time Strategy
Time strategy enables delaying the hydration of a component until a specified time (in milliseconds) has passed.
Never Hydrate
Never strategy prevents the component from being hydrated by Vue. This means that the component remains static and does not require Vue's hydration process.
If there's anything I may have overlooked or if you have any suggestions, please feel free to let me know. I truly appreciate your feedback.