Skip to content

Conversation

James-9696
Copy link
Collaborator

@James-9696 James-9696 commented Oct 10, 2025

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:
    在于$service的获取不到,导致控制台报错,添加兼容方式即可

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Bug Fixes
    • Improved service resolution in the Vue 3 adapter by using a VM-aware getter when available, falling back to the previous method otherwise. This enhances compatibility across setups and prevents cases where the service was not correctly resolved in certain component contexts. No API changes required for consumers. Ensures consistent behavior across applications.

@github-actions github-actions bot added the bug Something isn't working label Oct 10, 2025
Copy link

coderabbitai bot commented Oct 10, 2025

Walkthrough

Updated Vue 3 adapter’s tools() to resolve the service via root.$getService(vm) when available, otherwise use root.$service. No other API surfaces or exports changed.

Changes

Cohort / File(s) Summary
Vue 3 adapter service provisioning
packages/vue-common/src/adapter/vue3/index.ts
tools() now sets service via conditional resolution: prefer root.$getService(vm) if present; otherwise use root.$service. Public return value behavior changes at runtime; signatures remain unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor VM as Vue Component (vm)
  participant Adapter as Vue3 Adapter.tools()
  participant Root as App Root

  VM->>Adapter: tools(vm, root)
  alt root has $getService
    Adapter->>Root: $getService(vm)
    Root-->>Adapter: Service instance (vm-scoped)
  else fallback
    Adapter->>Root: access $service
    Root-->>Adapter: Service instance (root-scoped)
  end
  Adapter-->>VM: { service, ...otherTools }

  note over Adapter: Conditional service resolution introduced
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the code with whiskered grace,
A hop from root to vm’s embrace—
If getter’s there, I’ll fetch with glee,
Else root will pour my cup of tea.
Two paths, one service in my sight,
Thump-thump! The flow now feels just right. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title identifies a bugfix related to service retrieval but is phrased vaguely with redundant terms and does not clearly convey the specific change made in the Vue3 adapter; it could be more concise and specific to aid clear understanding. Consider revising the title to something more concise and specific, for example “fix: fallback to $service when $getService is unavailable in Vue3 adapter” to clearly reflect the change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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
  • Commit unit tests in branch fix-vue3-problem

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

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 44a4268 and 027ea56.

📒 Files selected for processing (1)
  • packages/vue-common/src/adapter/vue3/index.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/vue-common/src/adapter/vue3/index.ts (1)
packages/vue-hooks/src/vue-emitter.ts (1)
  • vm (15-49)
🔇 Additional comments (2)
packages/vue-common/src/adapter/vue3/index.ts (2)

378-378: LGTM! Backward-compatible service resolution.

The conditional logic correctly checks for $getService before calling it, falling back to $service when unavailable. This should resolve the console errors mentioned in the PR objectives.


378-379: Manually verify service return consistency and run demo tests

Existing Playwright specs under examples/sites/demos/.../custom-service.spec.ts cover components using service. Ensure both branches (root.$getService(vm) and fallback root.$service) expose the same API and produce no console errors when running these demos.

isMobileMode: mode === 'mobile',
service: root?.$service,
service: root?.$getService ? root?.$getService(vm) : root?.$service,
getService: () => root?.$getService(vm),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Missing existence check could cause runtime errors.

Unlike the service property at line 378, this getService function doesn't check if $getService exists before calling it. When $getService is unavailable, calling getService() will throw a TypeError: ... is not a function.

Apply this diff to add the same backward-compatibility check:

-    getService: () => root?.$getService(vm),
+    getService: () => root?.$getService ? root.$getService(vm) : root?.$service,
📝 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.

Suggested change
getService: () => root?.$getService(vm),
getService: () => root?.$getService ? root.$getService(vm) : root?.$service,
🤖 Prompt for AI Agents
In packages/vue-common/src/adapter/vue3/index.ts around line 379, the getService
function directly calls root?.$getService(vm) without ensuring $getService is
available; update it to perform the same backward-compatibility check used for
the service property at line 378 (e.g., check that root?.$getService exists and
is a function before calling it) and return undefined (or a no-op) when
$getService is not present so calling getService() cannot throw a TypeError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant