fix(custom-element) Assign custom element properties on prototype #13716
+59
−15
Conversation
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
WalkthroughThe changes introduce a new helper function to centralize prop accessor definitions on custom element prototypes, add a static property for async component definition tracking, and refine async resolution and prop handling logic. A new test verifies that subclassed custom elements can override property setters and affect the rendered output as expected. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CustomElement as SubclassedCustomElement
participant BaseElement as BaseCustomElement
User->>CustomElement: Set value = 999
CustomElement->>CustomElement: Overridden setter increments (999 → 1000)
CustomElement->>BaseElement: Call base setter with 1000
BaseElement->>CustomElement: Render with value 1000
sequenceDiagram
participant Browser as Browser
participant VueCustomElement as VueCustomElement
participant AsyncComponent as AsyncComponent
Browser->>VueCustomElement: Instantiate custom element
VueCustomElement->>AsyncComponent: Resolve async component
AsyncComponent-->>VueCustomElement: Return resolved definition
VueCustomElement->>VueCustomElement: Check and set asyncDef/static cache
VueCustomElement->>VueCustomElement: Call _defineProps on prototype (if needed)
VueCustomElement->>VueCustomElement: Delete instance prop shadows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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 (
|
mattheww-skyward
changed the title
mattheww-skyward
changed the title
mattheww-skyward
force-pushed
the
custom-element-properties
branch
from
cbfd888 to
8a0b714
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #13706
Fixing the tests raised a few questions I'm not sure of the answers to.
expose()? Should we go all the way up the prototype chain to VueElement?If we don't care at all we could overwrite
VueCustomElement.defwith the resolved definition instead of settingVueCustomElement.asyncDef. But we could have done that before - I'm not sure what Vue's stance on this sort of caching is.connectedCallback? My reflection call preserves the old behavior but it strikes me as kind of hacky.I added this check to fix the "handling properties set before upgrading" test failing, due to setting the corresponding attribute, and then, when mounted, assigning the property off of the (now stringified) attribute value. Maybe it should set the attribute plus a flag for backflow prevention? As a developer I found it somewhat surprising that properties and attributes behave differently before and after connection. But I don't know what is normal for custom elements.
Summary by CodeRabbit
New Features
Bug Fixes
Tests