refactor: replace onWatchCleanup and useTemplateRef api #348
Conversation
✅ Deploy Preview for antd-design-x-vue ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis update systematically replaces the deprecated or removed Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant VueWatcher
participant CleanupCallback
Component->>VueWatcher: watch(..., (newVal, oldVal, onCleanup) => { ... })
VueWatcher-->>Component: Triggers callback on change
Component->>CleanupCallback: Register cleanup via onCleanup(() => { ... })
VueWatcher-->>CleanupCallback: Calls cleanup function on invalidation/unmount
sequenceDiagram
participant Component
participant useTemplateRefPolyfill
participant VueInstance
Component->>useTemplateRefPolyfill: useTemplateRef('refKey')
useTemplateRefPolyfill->>VueInstance: Attach shallowRef to refs['refKey']
useTemplateRefPolyfill-->>Component: Return (readonly) shallowRef
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/examples-setup/sender/actions.vueOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser 'vue-eslint-parser' declared in '.eslintrc.js': Cannot find module 'vue-eslint-parser'
docs/examples-setup/sender/basic.vueOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser 'vue-eslint-parser' declared in '.eslintrc.js': Cannot find module 'vue-eslint-parser'
docs/examples/sender/basic.vueOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser 'vue-eslint-parser' declared in '.eslintrc.js': Cannot find module 'vue-eslint-parser'
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/bubble/BubbleList.vue (1)
49-49: Consider removing or implementing the commented useTemplateRef line.Since the PR objectives mention replacing
useTemplateRefAPI, this commented line might need to be addressed. IfuseTemplateRefis no longer needed, consider removing the comment.-// const listRef = useTemplateRef<HTMLDivElement>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/examples-setup/bubble/markdown.vue(2 hunks)docs/examples-setup/sender/actions.vue(2 hunks)docs/examples-setup/sender/basic.vue(2 hunks)docs/examples-setup/sender/sendStyle.vue(2 hunks)docs/examples/bubble/markdown.vue(2 hunks)docs/examples/sender/actions.vue(2 hunks)docs/examples/sender/basic.vue(2 hunks)docs/examples/sender/sendStyle.vue(2 hunks)src/_util/hooks/useTemplateRefPolyfill.ts(1 hunks)src/attachments/Attachments.vue(2 hunks)src/attachments/DropArea.vue(3 hunks)src/attachments/FileList/FileList.vue(2 hunks)src/attachments/FileList/FileListCard.vue(4 hunks)src/attachments/PlaceholderUploader.vue(1 hunks)src/attachments/SilentUploader.vue(1 hunks)src/bubble/Bubble.vue(1 hunks)src/bubble/BubbleList.vue(2 hunks)src/bubble/hooks/useTypedEffect.ts(2 hunks)src/sender/useSpeech.ts(3 hunks)
🔇 Additional comments (26)
src/bubble/Bubble.vue (1)
58-60:✅ Verification successful
Inconsistent migration approach compared to other files.
While the current
refusage works correctly, other files in this PR are using theuseTemplateRefpolyfill. The TODO comment mentions a readonly warning issue, but this should be verified to ensure the polyfill doesn't have the same problem.
🏁 Script executed:
#!/bin/bash # Check if other files successfully use the useTemplateRef polyfill without the readonly warning ast-grep --pattern 'useTemplateRef<$_>($_)'Length of output: 45
🏁 Script executed:
#!/bin/bash # Search for any usage of useTemplateRef across the repository rg -n "useTemplateRef" .Length of output: 2705
🏁 Script executed:
#!/bin/bash # Inspect the implementation of the useTemplateRef polyfill sed -n '1,200p' src/_util/hooks/useTemplateRefPolyfill.tsLength of output: 1350
🏁 Script executed:
#!/bin/bash # Display the first 200 lines of src/bubble/Bubble.vue for context sed -n '1,200p' src/bubble/Bubble.vueLength of output: 5609
🏁 Script executed:
#!/bin/bash # Search for divRef usage in the file to see its application in the template rg -n "divRef" src/bubble/Bubble.vueLength of output: 193
Ref polyfill not applicable in JSX—current
refapproach is correctThe
useTemplateRefpolyfill is designed to work with string-based<template ref="key">in SFCs, where it defines getters/setters on the component’srefsobject. In JSX/TSX (<div ref={divRef} />), Vue treats therefprop as an object ref and assigns the element directly todivRef.value. Since the polyfill wraps the underlyingshallowRefin areadonlyproxy in development, attempting to write toret.valuewill trigger the warning you observed.• In TSX files like
Bubble.vue, continue usingconst divRef = ref<HTMLDivElement>(null)
• You can safely remove the commented-outuseTemplateRefline and its TODO, or update the comment to note “polyfill only applies to template string refs, not JSX ref objects.”No further changes required here.
src/attachments/Attachments.vue (1)
3-3: LGTM! Clean migration to polyfill.The import changes correctly implement the migration from Vue's native
useTemplateRefto the local polyfill, maintaining the same API while potentially improving Vue version compatibility.Also applies to: 14-14
src/attachments/PlaceholderUploader.vue (1)
6-7: LGTM! Proper migration to polyfill.The import restructuring correctly moves
useTemplateRefto the local polyfill while maintaining other Vue imports. The implementation should remain functionally identical.src/attachments/SilentUploader.vue (1)
2-4:✅ Verification successful
LGTM! Consistent migration pattern.
The import changes correctly implement the migration strategy. Since this polyfill is now used across multiple components, ensure the polyfill implementation in
src/_util/hooks/useTemplateRefPolyfill.tshandles all edge cases properly.
🏁 Script executed:
#!/bin/bash # Verify the polyfill implementation exists and check its robustness cat src/_util/hooks/useTemplateRefPolyfill.tsLength of output: 1340
Approve: Consistent migration and robust polyfill implementation
I’ve reviewed
useTemplateRefPolyfill.tsand the implementation handles edge cases—duplicate key registration, missing component instance, and readonly wrapping in dev mode—gracefully. No further changes are needed here.docs/examples-setup/bubble/markdown.vue (1)
7-7: LGTM! Proper Vue 3 API modernization.The changes correctly replace the deprecated
onWatcherCleanupwith the standardonCleanupparameter provided bywatchEffect. This improves Vue compatibility while maintaining the same cleanup functionality.Also applies to: 26-33
docs/examples/sender/sendStyle.vue (1)
5-5: LGTM! Correct Vue 3 watcher cleanup implementation.The changes properly modernize the
watchcallback to use theonCleanupparameter instead of the deprecatedonWatcherCleanup. The timer cleanup logic is preserved while improving Vue API compliance.Also applies to: 17-27
docs/examples/bubble/markdown.vue (1)
7-7: LGTM! Consistent Vue 3 API modernization.The changes correctly implement the same
onWatcherCleanuptoonCleanupmigration pattern as other files, maintaining consistent cleanup behavior while using the proper Vue 3 API.Also applies to: 27-37
src/bubble/hooks/useTypedEffect.ts (1)
2-2: LGTM! Critical core library API modernization.This change in the core
useTypedEffecthook properly migrates fromonWatcherCleanupto the standard Vue 3onCleanupparameter. The timeout cleanup logic for the typing effect is preserved while improving Vue compatibility across the library.Also applies to: 36-46
src/attachments/DropArea.vue (2)
3-3: LGTM: Correct removal of deprecated import.The
onWatcherCleanupimport has been properly removed since it's now replaced by theonCleanupparameter in the watch callback.
31-31: LGTM: Proper migration to Vue 3 standard cleanup API.The watch callback has been correctly updated to use the
onCleanupparameter instead of the deprecatedonWatcherCleanup. The cleanup logic remains functionally equivalent, ensuring proper removal of drag event listeners.Also applies to: 57-62
docs/examples-setup/sender/actions.vue (2)
4-4: LGTM: Import cleanup is correct.The
onWatcherCleanupimport has been properly removed and replaced with the standard Vue 3 approach.
14-14: LGTM: Correct implementation of watcher cleanup.The migration to using the
onCleanupparameter from the watch callback is properly implemented. The timeout cleanup functionality remains identical while using the modern Vue 3 API.Also applies to: 22-24
docs/examples-setup/sender/basic.vue (2)
4-4: LGTM: Consistent import migration.The deprecated
onWatcherCleanupimport has been correctly removed.
14-14: LGTM: Proper watcher cleanup implementation.The watch callback correctly uses the
onCleanupparameter, and the timeout clearing logic remains functionally equivalent to the previous implementation.Also applies to: 20-22
docs/examples-setup/sender/sendStyle.vue (2)
5-5: LGTM: Import statement properly updated.The
onWatcherCleanupimport has been correctly removed as part of the Vue API migration.
15-15: LGTM: Consistent watcher cleanup migration.The implementation correctly uses the
onCleanupparameter from the watch callback, maintaining the same timeout cleanup functionality while following Vue 3 best practices.Also applies to: 21-23
src/bubble/BubbleList.vue (1)
65-70: LGTM: Correct migration to Vue's standard cleanup pattern.The watchEffect cleanup logic has been properly migrated from
onWatcherCleanupto the standardonCleanupcallback parameter. The initialization state management remains functionally equivalent.src/sender/useSpeech.ts (1)
56-80: LGTM: Proper cleanup migration with memory leak prevention.The watchEffect has been correctly updated to use the
onCleanupcallback parameter instead ofonWatcherCleanup. The cleanup logic properly prevents memory leaks by nullifying the permission statusonchangehandler when the effect is invalidated.docs/examples/sender/actions.vue (1)
15-26: LGTM: Correct watch cleanup pattern implementation.The watch function has been properly updated to use the
onCleanupcallback parameter instead ofonWatcherCleanup. The timer cleanup logic is correctly preserved, ensuring timeouts are cleared when the watcher is invalidated.docs/examples/sender/basic.vue (1)
15-25: LGTM: Consistent cleanup pattern implementation.The watch function cleanup has been correctly migrated to use the
onCleanupcallback parameter. The timer cleanup logic is properly preserved, maintaining the same behavior while using Vue's standard cleanup mechanism.src/attachments/FileList/FileList.vue (2)
3-3: LGTM! Import updates align with Vue API modernization.The removal of deprecated
onWatcherCleanupand adoption of the localuseTemplateRefpolyfill properly supports the downward compatibility objectives.Also applies to: 8-8
40-46: Correct migration to onCleanup callback pattern.The
watchEffectproperly uses theonCleanupcallback parameter instead of the deprecatedonWatcherCleanupAPI. The logic remains functionally equivalent.src/attachments/FileList/FileListCard.vue (2)
4-4: LGTM! Consistent import modernization.The import changes mirror the pattern from other files in this refactor, correctly adopting the polyfilled
useTemplateRefand removing deprecated APIs.Also applies to: 13-13
165-179: Proper cleanup pattern implementation.The
watchcorrectly uses theonCleanupcallback to manage thesyncedflag, preventing memory leaks and stale state updates during asynchronous operations.src/_util/hooks/useTemplateRefPolyfill.ts (2)
1-11: Well-structured polyfill setup with proper Vue internals usage.The imports and development mode detection follow Vue's internal patterns. The
EMPTY_OBJconstant correctly provides an immutable reference for comparison.
14-40: Robust useTemplateRef implementation with comprehensive error handling.The polyfill correctly:
- Validates component instance context
- Handles existing property descriptors
- Provides helpful development warnings
- Uses proper property descriptor configuration
| const ret = __DEV__ ? readonly(r) : r | ||
| if (__DEV__) { | ||
| knownTemplateRefs.add(ret) | ||
| } | ||
| return ret | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Appropriate development mode safety measures.
The readonly wrapping in development mode and WeakSet tracking provide good developer experience while maintaining performance in production.
Verify that this polyfill is used consistently across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check that useTemplateRef imports are consistently using the polyfill
# Search for any remaining direct Vue useTemplateRef imports
echo "=== Checking for direct Vue useTemplateRef imports ==="
rg "import.*useTemplateRef.*from ['\"]vue['\"]" --type ts --type vue
# Search for polyfill usage
echo "=== Checking polyfill usage ==="
rg "import.*useTemplateRef.*from.*useTemplateRefPolyfill" --type ts --type vueLength of output: 403
🏁 Script executed:
#!/bin/bash
# Re-run import checks across TS and Vue files without relying on rg file types
echo "=== Checking for direct Vue useTemplateRef imports ==="
rg -g '*.ts' -g '*.tsx' -g '*.vue' "import.*useTemplateRef.*from ['\"][^'\"]*vue['\"]"
echo "=== Checking polyfill usage ==="
rg -g '*.ts' -g '*.tsx' -g '*.vue' "import.*useTemplateRef.*from.*useTemplateRefPolyfill"Length of output: 1015
Inconsistent polyfill import detected
One example file still pulls useTemplateRef directly from Vue instead of the polyfill:
• docs/examples/attachments/basic.vue
- import { ref, computed, useTemplateRef } from 'vue'
+ import { ref, computed } from 'vue'
+ import { useTemplateRef } from '../../../src/_util/hooks/useTemplateRefPolyfill'Please update this and any other occurrences to use the polyfill.
📝 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 ret = __DEV__ ? readonly(r) : r | |
| if (__DEV__) { | |
| knownTemplateRefs.add(ret) | |
| } | |
| return ret | |
| } | |
| - import { ref, computed, useTemplateRef } from 'vue' | |
| + import { ref, computed } from 'vue' | |
| + import { useTemplateRef } from '../../../src/_util/hooks/useTemplateRefPolyfill' |
🤖 Prompt for AI Agents
In src/_util/hooks/useTemplateRefPolyfill.ts around lines 41 to 46, the code
correctly implements the polyfill with development mode safety measures.
However, the issue is that elsewhere in the codebase, such as in
docs/examples/attachments/basic.vue, useTemplateRef is imported directly from
Vue instead of this polyfill. To fix this, search the entire codebase for any
imports of useTemplateRef directly from 'vue' and replace them with imports from
the polyfill module to ensure consistent usage.
| @@ -0,0 +1,46 @@ | |||
| import { type ShallowRef, readonly, shallowRef, getCurrentInstance } from 'vue' | |||
There was a problem hiding this comment.
hi，这段useTemplateRefPolyfill代码是原创或是引用自哪里吗？
我未在vue相关文档中了解到此代码，是否可以使用vue文档中提到的写法来代替useTemplateRef？
There was a problem hiding this comment.
useTemplateRef代码是基于vue仓库的，hooks的具体实现不是很复杂就实现了一个。如果需要改为同名ref的形式，我再提交一下修改。
There was a problem hiding this comment.
看起来vue源码中也仅是语法糖的封装，这个复刻实现是可行的👍
|
另一个让我纠结的点是，我们是否有必要将文档demo的写法也都改成vue3.3？我觉得文档中可以保留vue3.5的写法，毕竟这也是vue推荐的写法。 |
在改代码的时候我也注意到了这个问题，我觉得两种都是可行的，因为最小改动的原因就没有修改demo中的写法。 我个人倾向是可以修改成3.3的写法，毕竟新的api目前来说使用并不广范，按照大部分使用vue的开发者的视角，阅读文档的疑问会少些，而且修改的demo的工作量应该不会特别大 |
Agree! |
2 participants
#187 替换
onWatchCleanupuseTemplateRef实现vue向下兼容Summary by CodeRabbit
New Features
Refactor
Chores