Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
@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: |
Size ReportBundles
Usages
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/runtime-vapor/__tests__/components/Transition.spec.ts (1)
13-76: Add one async-wrapper regression case here.These assertions only hit the normal component branch.
resolveTransitionBlock()now has the same key fallback duplicated under the resolvedisAsyncWrapper(block)path, so that branch can still drift without tripping this suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/runtime-vapor/__tests__/components/Transition.spec.ts` around lines 13 - 76, Add a test that exercises the isAsyncWrapper path of resolveTransitionBlock by wrapping the Child in an async-wrapper component and asserting the same key-fallback behavior: create a Child via defineVaporComponent, create an async wrapper component (so the child's block is wrapped as an async wrapper), call createComponent/define to render the wrapper, set an explicit key with setBlockKey on the inner child, clear child.block.$key and then call resolveTransitionBlock(wrapperInstance) and assert the resolved.$key prefers the explicit key (and add similar assertions for fallback to uid and preserving falsy keys if desired); this ensures the duplicated key-fallback logic inside the isAsyncWrapper branch is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/runtime-vapor/src/components/Transition.ts`:
- Around line 371-378: The code currently only falls back when child.$key ===
undefined, which preserves a literal null key; change the checks to use nullish
semantics (child.$key == null) so both null and undefined are treated as “no
key” and then assign child.$key = block.$key ?? block.uid; update both
occurrences (the branch that assigns child.$key and the similar block at the
later occurrence) and ensure transitionTypeMap.set(child, block.type) remains
unchanged.
In `@packages/runtime-vapor/src/helpers/setKey.ts`:
- Around line 16-19: The array branch in setKey (in
packages/runtime-vapor/src/helpers/setKey.ts) ignores multi-root blocks causing
keys not to propagate; update the isArray(block) branch to call setBlockKey for
every element when block.length > 1 (i.e., iterate over block and call
setBlockKey(item, key) for each child) instead of only handling length === 1 so
all elements receive the key; ensure any existing early-return logic or
surrounding control flow in setKey and the setBlockKey helper remains consistent
after adding the loop.
---
Nitpick comments:
In `@packages/runtime-vapor/__tests__/components/Transition.spec.ts`:
- Around line 13-76: Add a test that exercises the isAsyncWrapper path of
resolveTransitionBlock by wrapping the Child in an async-wrapper component and
asserting the same key-fallback behavior: create a Child via
defineVaporComponent, create an async wrapper component (so the child's block is
wrapped as an async wrapper), call createComponent/define to render the wrapper,
set an explicit key with setBlockKey on the inner child, clear child.block.$key
and then call resolveTransitionBlock(wrapperInstance) and assert the
resolved.$key prefers the explicit key (and add similar assertions for fallback
to uid and preserving falsy keys if desired); this ensures the duplicated
key-fallback logic inside the isAsyncWrapper branch is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cae63d7f-938a-44d9-9c5a-bde1a2e77d59
⛔ Files ignored due to path filters (1)
packages/compiler-vapor/__tests__/transforms/__snapshots__/transformKey.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (37)
packages-private/vapor-e2e-test/__tests__/transition-group.spec.tspackages-private/vapor-e2e-test/__tests__/transition.spec.tspackages-private/vapor-e2e-test/transition-group/cases/interop/avoid-set-transition-hooks-for-comment-node.vuepackages-private/vapor-e2e-test/transition-group/cases/interop/static-keyed-vdom-component-enter.vuepackages-private/vapor-e2e-test/transition-group/cases/interop/unkeyed-vdom-component-update.vuepackages-private/vapor-e2e-test/transition-group/cases/vapor-transition-group/static-keyed-component-enter.vuepackages-private/vapor-e2e-test/transition-group/cases/vapor-transition-group/static-keyed-enter.vuepackages-private/vapor-e2e-test/transition-group/components/StaticKeyItem.vuepackages-private/vapor-e2e-test/transition-group/components/VdomCommentToggle.vuepackages-private/vapor-e2e-test/transition-group/components/VdomKeyItem.vuepackages-private/vapor-e2e-test/transition/cases/interop/should-work-with-static-keyed-vdom-component.vuepackages-private/vapor-e2e-test/transition/cases/keyed/should-work-with-static-keyed-component.vuepackages-private/vapor-e2e-test/transition/cases/keyed/should-work-with-static-keyed-element.vuepackages-private/vapor-e2e-test/transition/components/StaticKeyItem.vuepackages-private/vapor-e2e-test/transition/components/VdomKeyItem.vuepackages/compiler-vapor/__tests__/transforms/transformKey.spec.tspackages/compiler-vapor/__tests__/transforms/vIf.spec.tspackages/compiler-vapor/src/generators/key.tspackages/compiler-vapor/src/generators/operation.tspackages/compiler-vapor/src/ir/index.tspackages/compiler-vapor/src/transforms/transformElement.tspackages/compiler-vapor/src/utils.tspackages/runtime-vapor/__tests__/apiCreateDynamicComponent.spec.tspackages/runtime-vapor/__tests__/components/KeepAlive.spec.tspackages/runtime-vapor/__tests__/components/Transition.spec.tspackages/runtime-vapor/__tests__/components/TransitionGroup.spec.tspackages/runtime-vapor/__tests__/helpers/setKey.spec.tspackages/runtime-vapor/__tests__/vdomInterop.spec.tspackages/runtime-vapor/src/block.tspackages/runtime-vapor/src/component.tspackages/runtime-vapor/src/components/KeepAlive.tspackages/runtime-vapor/src/components/Transition.tspackages/runtime-vapor/src/components/TransitionGroup.tspackages/runtime-vapor/src/fragment.tspackages/runtime-vapor/src/helpers/setKey.tspackages/runtime-vapor/src/index.tspackages/runtime-vapor/src/vdomInterop.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
setBlockKeypublic API for explicit key management in componentsTests