fix(runtime-core): avoid setting direct ref of useTemplateRef in dev #13449
Conversation
WalkthroughThe changes introduce new test cases for Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant Renderer
participant Ref
Component->>Renderer: Render with useTemplateRef(ref, ref_key)
Renderer->>Renderer: Check canSetRef(ref)
alt canSetRef is true
Renderer->>Ref: Set ref.value = element
else canSetRef is false
Renderer-->>Ref: Skip setting ref.value
end
Renderer->>Component: Continue rendering
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)packages/runtime-core/src/rendererTemplateRef.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (11)
✨ Finishing Touches
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 (
|
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
| @@ -3082,6 +3082,7 @@ describe('e2e: Transition', () => { | |||
|
|
|||
| // enter | |||
| await classWhenTransitionStart() | |||
| await nextFrame() | |||
There was a problem hiding this comment.
Unrelated to the goal of this PR, but this test was flaky for me locally (even before the change); I see in most other places in this file nextFrame is called before transitionFinish so I added it here as well but please let me know if adding it here is incorrect
There was a problem hiding this comment.
I don't think this change is necessary. the e2e tests have passed on ci.
There was a problem hiding this comment.
Yeah fair enough, I was having some flakiness running this test locally though, even on the main branch with no code changes :/
My best guess is that the 200ms timeout used on transitionFinish in the CI environment is lenient enough to consistently pass but the 50ms timeout locally is what was causing flakiness for me. If I double the waiting period for the transitionFinish call in this test it also begins passing consistently locally for example.
I can revert this change if you'd like though
There was a problem hiding this comment.
The PR should fix the intended bug only and not introduce unrelated changes.
|
/ecosystem-ci run |
edison1105
added
the
🔨 p3-minor-bug
|
📝 Ran ecosystem CI: Open
|
Avoid directly setting the ref value in dev mode whenever a ref object from a
useTemplateRefcall is used for rendering, updating only the component owner refs object instead (whichuseTemplateRefis hooked up to correctly update from).When code like the following:
is compiled in inline mode the output would contain:
which then attempts to update the
myRefobject directly, showing a warning:Set operation on key "value" failed: target is readonly.; this change should avoid this warning from showing.closes #12852
Summary by CodeRabbit