fix(runtime-core): improve consistency of `PublicInstanceProxyHandlers.has`
While working on a larger refactor I noticed some small inconsistencies between the get and has handlers for PublicInstanceProxyHandlers. This PR tries to make has consistent with get.
In particular:
- Properties in
datathat begin with$are not proxied through the instance. Thehashandler was not taking that into account. Example. - CSS modules expose properties from
__cssModules. Thehashandler wasn't considering these.
I've also added tests for a handful of other related cases, mostly involving the use of the $ prefix.
A few notes...
- Both
propsandsetupshow warnings when creating properties that begin with$, so I haven't attempted to 'fix' those. - The correct handling of the prefix
_is currently unclear. Butgetandhasboth handle it the same way, so I haven't changed that. - My change uses
key[0], which assumes thekeyis astring. It could also be asymbol, but it will still work correctly and this is consistent with how it is handled in thegethandler. - The check for CSS modules is not using
hasOwn. Maybe it should be, but I was just aiming for consistency withget.
I'm hoping to open another PR in the near future that refactors this code further to allow get and has to share more code, but I wanted to get this fixed first.
Summary by CodeRabbit
- Bug Fixes
- Improved detection of properties (including computed, CSS module, and global properties) on component instances, ensuring more consistent and expected behavior when using the "in" operator.
- Tests
- Enhanced test coverage for component instance property checks, including new scenarios for reactive sources, computed properties, CSS modules, and global properties.
Walkthrough
The changes update the logic of the component proxy's "has" trap to refine how properties are checked for existence, particularly handling $-prefixed keys and CSS module properties. Corresponding tests were enhanced to verify the new behaviors, including checks for computed properties, global properties, and CSS modules.
Changes
| File(s) | Change Summary |
|---|---|
| packages/runtime-core/src/componentPublicInstance.ts | Refined the "has" trap logic to ignore $-prefixed keys in data and include CSS module properties in type checks. |
| packages/runtime-core/tests/componentPublicInstance.spec.ts | Enhanced tests to cover new behaviors for $-prefixed keys, computed properties, global properties, and CSS modules. |
Sequence Diagram(s)
sequenceDiagram
participant Test as Test Suite
participant Proxy as Component Proxy
participant Data as data
participant Computed as computed
participant Type as component type (__cssModules)
participant Global as globalProperties
Test->>Proxy: "key" in proxy
alt key exists in setupState, data (not $-prefixed), props, attrs
Proxy-->>Test: true
else key exists in computed, globalProperties
Proxy-->>Test: true
else key exists in type.__cssModules
Proxy-->>Test: true
else
Proxy-->>Test: false
end
Poem
In the warren of code, a proxy stands tall,
Checking keys—big, small, and those with a $ call.
CSS modules now join the proxy’s keen sight,
While tests hop along, ensuring all is right.
A bunny’s delight in a type-safe spring,
With every key checked, let the happy code sing!
🐇✨
📜 Recent review details
Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 5f8314cb7f4b25efcc189987639bd9ac7f171926 and 2576fbdb7057eda88a802f251c9bb7228fe32b56.
📒 Files selected for processing (2)
packages/runtime-core/__tests__/componentPublicInstance.spec.ts(2 hunks)packages/runtime-core/src/componentPublicInstance.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test / e2e-test
🔇 Additional comments (9)
packages/runtime-core/src/componentPublicInstance.ts (3)
578-578: Includetypeproperty for CSS modules support.Good addition of the
typeproperty to the destructuring, which is needed for the CSS modules check added later in thehasmethod.
582-582: Improve variable declarations for clarity.The variable declarations are well-structured and follow the existing pattern in the codebase.
583-592: Excellent consistency improvements betweengetandhashandlers.The changes successfully address the inconsistencies mentioned in the PR objectives:
$-prefixed data properties: The condition
key[0] !== '$'correctly excludes data properties starting with$from thehascheck, matching thegethandler behavior.CSS modules support: The CSS modules check
((cssModules = type.__cssModules) && cssModules[key])properly adds support for CSS module properties, which was previously missing from thehashandler.Boolean return: The double negation
!!ensures a proper boolean return value for thehastrap.These changes improve the consistency between the proxy handlers as intended.
packages/runtime-core/__tests__/componentPublicInstance.spec.ts (6)
170-170: Good test data for $-prefixed property exclusion.Adding
$foo: 0to the data object provides the necessary test case for verifying that $-prefixed data properties are excluded from thehascheck.
173-180: Clever computed property test implementation.The computed properties that throw errors ensure the test validates the
hasbehavior without accidentally triggering the getter evaluation, which is exactly what we want to test.
186-189: Proper CSS modules test setup.The
__cssModulesobject with$styleandcssStylesproperties provides comprehensive coverage for the CSS modules functionality added to thehashandler.
197-197: Comprehensive test coverage for all behavioral changes.The test assertions thoroughly validate all the changes made to the
hashandler:
$foois correctly excluded from data presence checks (line 205)- Computed properties are properly detected (lines 209-210)
- CSS module properties are correctly recognized (lines 214-215)
- Global properties including $-prefixed ones work as expected (line 218)
This provides excellent coverage of the new behavior.
Also applies to: 205-205, 209-218
227-235: Excellent edge case testing.The tests for non-existent properties and property assignment scenarios ensure the
hasbehavior works correctly in edge cases, including:
- Accessing non-existent $-prefixed properties doesn't affect
haschecks- Setting properties correctly updates the
hasbehaviorThese edge cases are important for ensuring robust behavior.
243-246: Updated Object.keys expectations align with changes.The updated
Object.keystest correctly includes the new computed and assigned properties (cmp,$cmp,$baz), ensuring the enumerable properties match the expected behavior after thehashandler changes.
✨ Finishing Touches
- [ ] 📝 Generate Docstrings
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.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Explain this complex logic.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai explain this code block.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and explain its main purpose.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Support
Need 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)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
Size Report
Bundles
| File | Size | Gzip | Brotli |
|---|---|---|---|
| runtime-dom.global.prod.js | 101 kB (+49 B) | 38.4 kB (+22 B) | 34.6 kB (+9 B) |
| vue.global.prod.js | 159 kB (+49 B) | 58.6 kB (+22 B) | 52.2 kB (-18 B) |
Usages
| Name | Size | Gzip | Brotli |
|---|---|---|---|
| createApp (CAPI only) | 46.7 kB (+49 B) | 18.2 kB (+21 B) | 16.7 kB (+20 B) |
| createApp | 54.6 kB (+49 B) | 21.3 kB (+22 B) | 19.4 kB (+9 B) |
| createSSRApp | 58.9 kB (+49 B) | 23 kB (+26 B) | 21 kB (+22 B) |
| defineCustomElement | 59.6 kB (+49 B) | 22.8 kB (+27 B) | 20.8 kB (+23 B) |
| overall | 68.7 kB (+49 B) | 26.4 kB (+23 B) | 24.1 kB (+4 B) |
@vue/compiler-core
npm i https://pkg.pr.new/@vue/compiler-core@13507
@vue/compiler-dom
npm i https://pkg.pr.new/@vue/compiler-dom@13507
@vue/compiler-sfc
npm i https://pkg.pr.new/@vue/compiler-sfc@13507
@vue/compiler-ssr
npm i https://pkg.pr.new/@vue/compiler-ssr@13507
@vue/reactivity
npm i https://pkg.pr.new/@vue/reactivity@13507
@vue/runtime-core
npm i https://pkg.pr.new/@vue/runtime-core@13507
@vue/runtime-dom
npm i https://pkg.pr.new/@vue/runtime-dom@13507
@vue/server-renderer
npm i https://pkg.pr.new/@vue/server-renderer@13507
@vue/shared
npm i https://pkg.pr.new/@vue/shared@13507
vue
npm i https://pkg.pr.new/vue@13507
@vue/compat
npm i https://pkg.pr.new/@vue/compat@13507
commit: f2ad41c
/ecosystem-ci run
📝 Ran ecosystem CI: Open
| suite | result | latest scheduled |
|---|---|---|
| nuxt | :white_check_mark: success | :white_check_mark: success |
| language-tools | :white_check_mark: success | :white_check_mark: success |
| router | :white_check_mark: success | :white_check_mark: success |
| vue-i18n | :white_check_mark: success | :white_check_mark: success |
| vant | :white_check_mark: success | :white_check_mark: success |
| primevue | :white_check_mark: success | :white_check_mark: success |
| radix-vue | :white_check_mark: success | :white_check_mark: success |
| test-utils | :white_check_mark: success | :white_check_mark: success |
| vitepress | :white_check_mark: success | :white_check_mark: success |
| vite-plugin-vue | :white_check_mark: success | :white_check_mark: success |
| quasar | :white_check_mark: success | :white_check_mark: success |
| vue-macros | :x: failure | :x: failure |
| vuetify | :white_check_mark: success | :white_check_mark: success |
| vueuse | :white_check_mark: success | :white_check_mark: success |
| vue-simple-compiler | :white_check_mark: success | :white_check_mark: success |
| pinia | :white_check_mark: success | :white_check_mark: success |