refactor(runtime-core): check props rather than propsOptions[0]
#13514
Conversation
WalkthroughReplaced checks against cached Changes
Sequence Diagram(s)sequenceDiagram
participant Proxy as Component Proxy
participant Instance as Component Instance
participant Props as instance.props
rect rgb(230, 245, 255)
Note over Proxy,Instance: get / has trap flow (new)
end
Proxy->>Instance: trap.get(key)
alt key in data / setupState / ctx
Instance-->>Proxy: return resolved value
else check props directly
Instance->>Props: hasOwn(props, key)?
alt true
Props-->>Proxy: return props[key]
else
Instance-->>Proxy: fallback lookup / undefined
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)packages/runtime-core/src/componentPublicInstance.ts (1)
⏰ 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). (1)
🔇 Additional comments (2)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
@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: |
Size ReportBundles
Usages
|
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
edison1105
added
ready to merge
# Conflicts: # packages/runtime-core/src/componentPublicInstance.ts
4 participants
This is minor refactoring of the logic for handling props in
PublicInstanceProxyHandlers.getandPublicInstanceProxyHandlers.has, switching it from checkingpropsOptions[0]to checkingprops. The new version is simpler and leads to a smaller build.In Vue 3.0, props could be missing from the
propsobject. Props were only included if they were passed in by the parent or had a default value. The logic inPublicInstanceProxyHandlerscouldn't rely on checkingprops, so it checkedpropsOptions[0]instead.That changed in 3.1.0. See #3288 for more details, but
propsnow contains all the props.Some other notes about what I changed and why...
sethasn't been changed as that was already usingprops.EMPTY_OBJ(as used for other sources of properties, e.g.data), butpropswon't be anEMPTY_OBJby the time this code runs, even for components that don't declare any props.propsOptionsis populated beforeprops, so there is potentially a timing issue introduced by this change. However, in practice, the only user-facing 'hook' that runs between the population of those two objects is thedefaultfunction for a prop. Thedefaultfunction doesn't have access to the relevant proxy, so it could only encounter a problem if it calledgetCurrentInstance().proxy(which it shouldn't be doing anyway) and then did some very strange things with it. I don't think this is a realistic case.ctx(seeexposePropsOnRenderContextin the same file). Even if the logic for props is wrong it still 'works' because they're accessed viactxinstead. If the logic were wrong it would likely only fail in a production build.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.