-
-
Notifications
You must be signed in to change notification settings - Fork 9k
feat(runtime-vapor): support setup fn and render fn co-usage for defineVaporComponent #14179
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
base: minor
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/compiler-vapor
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/runtime-vapor
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
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.
Pull request overview
This PR enables defineVaporComponent to support concurrent use of both setup and render functions, allowing components to return state from setup that can be consumed by a render function. Previously, the setup function parameter was explicitly checked, but the new implementation uses a more flexible condition based on whether the setup result is a block.
Key changes:
- Removed the
setupFnparameter fromhandleSetupResultfunction - Modified the production code path to handle setup functions that return non-block state alongside render functions
- Updated conditional logic from checking
!setupFnto checking!isBlock(setupResult)for determining when to call the render function
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ErrorCodes.RENDER_FUNCTION, | ||
| ) | ||
| // support setup fn and render fn co-usage for defineComponent expose | ||
| if (!isBlock(setupResult) && component.render) { |
Copilot
AI
Dec 8, 2025
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.
Missing devtoolsRawSetupState assignment for production devtools support.
The dev-only path at lines 965-966 sets instance.devtoolsRawSetupState when __DEV__ || __FEATURE_PROD_DEVTOOLS__ is true. This production code path should have the same assignment to ensure devtools work correctly in production builds with devtools enabled.
Suggestion: Add before line 979:
if (__DEV__ || __FEATURE_PROD_DEVTOOLS__) {
instance.devtoolsRawSetupState = setupResult
}| if (!isBlock(setupResult) && component.render) { | |
| if (!isBlock(setupResult) && component.render) { | |
| if (__DEV__ || __FEATURE_PROD_DEVTOOLS__) { | |
| instance.devtoolsRawSetupState = setupResult | |
| } |
| if (!isBlock(setupResult) && component.render) { | ||
| instance.setupState = setupResult | ||
| instance.block = | ||
| callWithErrorHandling( | ||
| component.render, | ||
| instance, | ||
| ErrorCodes.RENDER_FUNCTION, | ||
| [ | ||
| instance.setupState, | ||
| instance.props, | ||
| instance.emit, | ||
| instance.attrs, | ||
| instance.slots, | ||
| ], | ||
| ) || [] |
Copilot
AI
Dec 8, 2025
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.
Missing test coverage for the new setup + render co-usage functionality in production mode.
While there's an existing test at line 420 in component.spec.ts that tests setup + render together, it only verifies dev warnings for non-existent property access. The new production code path (lines 978-992) lacks test coverage for:
- Verifying that setupState properties (especially refs) are properly accessible and unwrapped in the render function in production mode
- Testing that the render function receives the correct parameters in the expected order
- Ensuring devtools integration works correctly with
devtoolsRawSetupState
Consider adding a test that:
- Sets
__DEV__ = false - Creates a component with both setup (returning refs/reactive state) and render functions
- Verifies the render function can access and use the setup state correctly
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.