Skip to content

Conversation

skirtles-code
Copy link
Contributor

@skirtles-code skirtles-code commented Oct 5, 2025

This is a minor refactoring that adds __FEATURE_OPTIONS_API__ to places where PublicInstanceProxyHandlers uses data. For builds using __VUE_OPTIONS_API__: false this should allow the code to be removed from production builds.

Usages of data inside dev-only code haven't been changed and shouldn't be impacted.

This is a small part of a larger refactoring I'm attempting, which also includes #13507 and #13514. That should eventually allow the switch/case logic to be replaced. For now, the usage of data inside the switch is retained as there isn't a convenient way to remove it.

Summary by CodeRabbit

  • Refactor
    • Gated component data property access, mutation, and existence checks behind the Options API feature flag.
    • Ensures behavior aligns with builds: when the Options API is disabled, data properties are not exposed via instance proxies; when enabled, behavior is unchanged.

Copy link

coderabbitai bot commented Oct 5, 2025

Walkthrough

Adds FEATURE_OPTIONS_API guards to data-related branches in PublicInstanceProxyHandlers within componentPublicInstance.ts, conditioning get, set, and has behavior on the options API feature flag.

Changes

Cohort / File(s) Summary of changes
Runtime core: PublicInstanceProxyHandlers guards
packages/runtime-core/src/componentPublicInstance.ts
Wrapped data-backed get/set/has paths with FEATURE_OPTIONS_API checks; data property access, mutation, and existence checks now occur only when the options API feature flag is enabled.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User as User Code
  participant Proxy as PublicInstanceProxyHandlers
  participant Inst as Component Instance
  participant Data as instance.data
  participant Flag as __FEATURE_OPTIONS_API__

  rect rgb(245,248,255)
  note over User,Proxy: Property read (get)
  User->>Proxy: access key
  Proxy->>Flag: check enabled?
  alt Flag enabled AND key in Data
    Proxy->>Data: return Data[key]
  else
    Proxy->>Inst: fallback to other sources
  end
  end

  rect rgb(245,255,245)
  note over User,Proxy: Property write (set)
  User->>Proxy: set key = value
  Proxy->>Flag: check enabled?
  alt Flag enabled AND key in Data
    Proxy->>Data: Data[key] = value
    Proxy-->>User: true
  else
    Proxy->>Inst: other mutation paths
  end
  end

  rect rgb(255,248,245)
  note over User,Proxy: Existence check (has)
  User->>Proxy: "key" in proxy
  Proxy->>Flag: check enabled?
  alt Flag enabled AND key in Data AND not startsWith("$")
    Proxy-->>User: true
  else
    Proxy->>Inst: check other sources
  end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

ready to merge

Poem

I thump my paws in tidy glee,
A flag now guards the data tree—
Get, set, has, they mind the gate,
Options on? Proceed—else wait.
In burrows of control and flow,
The proxies learn just when to go. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change by indicating that the runtime-core module now checks the feature flag when forwarding data properties, which matches the refactor performed in the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 c16f8a9 and 68fb26d.

📒 Files selected for processing (1)
  • packages/runtime-core/src/componentPublicInstance.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-core/src/componentPublicInstance.ts (1)
packages/shared/src/general.ts (2)
  • EMPTY_OBJ (3-5)
  • hasOwn (34-37)
⏰ 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)
  • GitHub Check: Redirect rules
  • GitHub Check: Header rules
  • GitHub Check: Pages changed
🔇 Additional comments (3)
packages/runtime-core/src/componentPublicInstance.ts (3)

451-457: LGTM! Feature flag correctly guards data access.

The addition of __FEATURE_OPTIONS_API__ before the data checks enables proper tree-shaking when the Options API is disabled. The condition ordering (feature flag → EMPTY_OBJ check → hasOwn) is optimal for both compile-time elimination and runtime performance.


552-558: LGTM! Feature flag correctly guards data mutation.

Consistent with the get handler, this change properly gates data property mutations behind the Options API feature flag, enabling tree-shaking when the feature is disabled.


593-596: LGTM! Feature flag correctly guards data existence check.

The parenthesized condition group ensures all four checks (feature flag, EMPTY_OBJ, reserved prefix, and hasOwn) are properly AND-ed together before being OR-ed with other conditions. This maintains correct logic while enabling tree-shaking.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

github-actions bot commented Oct 5, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB 38.6 kB 34.8 kB
vue.global.prod.js 160 kB 58.7 kB 52.2 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.6 kB (-89 B) 18.2 kB (-25 B) 16.7 kB (-29 B)
createApp 54.7 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 60 kB 23 kB 21 kB
overall 68.8 kB 26.5 kB 24.2 kB

Copy link

pkg-pr-new bot commented Oct 5, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13966

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13966

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13966

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13966

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13966

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13966

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13966

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13966

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13966

vue

npm i https://pkg.pr.new/vue@13966

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13966

commit: 68fb26d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant