docs: clarify stubActions limitations in Setup stores#3121
docs: clarify stubActions limitations in Setup stores#3121dennybiasiolli wants to merge 1 commit intovuejs:v4from
stubActions limitations in Setup stores#3121Conversation
📝 WalkthroughWalkthroughAdded a documentation section that explains how Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/testing/src/testing.spec.ts (1)
571-626: Consider grouping the three scenario tests under a shareddescribeblock.These three tests form a cohesive unit (Options intercept, Setup limitation, Setup workaround) that documents
stubActionsbehavior on inner calls. Wrapping them in something likedescribe('inner action stubbing', () => { ... })would improve test output readability and make the documented contract easier to navigate. Also, the assertions and rationale look correct and match the JSDoc note intesting.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testing/src/testing.spec.ts` around lines 571 - 626, Group the three related tests that exercise inner action stubbing into a shared describe block (e.g., describe('inner action stubbing', () => { ... })) to improve test output and organization: move the it blocks that reference useCounter, useCounterSetup, and the inline useStore (the tests titled "stubs inner actions in Options Store", "can not stub inner actions called via closed-over references in Setup Store (limitation)", and "stubs inner actions called via store proxy in Setup Store (workaround)") inside a single describe, preserving their existing setup calls to createTestingPinia, setActivePinia, and assertions so behavior and JSDoc alignment remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/testing/src/testing.spec.ts`:
- Around line 571-626: Group the three related tests that exercise inner action
stubbing into a shared describe block (e.g., describe('inner action stubbing',
() => { ... })) to improve test output and organization: move the it blocks that
reference useCounter, useCounterSetup, and the inline useStore (the tests titled
"stubs inner actions in Options Store", "can not stub inner actions called via
closed-over references in Setup Store (limitation)", and "stubs inner actions
called via store proxy in Setup Store (workaround)") inside a single describe,
preserving their existing setup calls to createTestingPinia, setActivePinia, and
assertions so behavior and JSDoc alignment remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45154ee6-b2e9-4825-a596-cf00c70ced39
📒 Files selected for processing (2)
packages/testing/src/testing.spec.tspackages/testing/src/testing.ts
dennybiasiolli
force-pushed
the
clarify-actions-stubbing
branch
from
2bc7c9f to
40d1e2e
Compare
|
Tests updated after this suggestion
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/testing/src/testing.spec.ts (1)
602-627: Workaround test looks good; consider reusinguseCounterSetupfor symmetry.The test correctly shows that routing inner calls through
useStore().increment()makes stubbing effective. As an optional nit, defining a freshuseStorehere duplicates fixture setup and slightly diverges from the first two tests which reuseuseCounter/useCounterSetup. You could add adoubleIncrementWithSelftouseCounterSetupinstead, keeping all three scenarios on the same fixture — but since the inline store keeps the workaround self-contained and easy to read, this is purely a style preference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/testing/src/testing.spec.ts` around lines 602 - 627, Replace the inline store in the test with the existing fixture by adding a doubleIncrementWithSelf action to the useCounterSetup fixture (or to useCounter if that’s the setup used elsewhere) and then call useCounterSetup() (or useCounter()) in this test; update the test to reference the fixture’s doubleIncrementWithSelf and its increment so the behavior and assertions remain the same while reusing the shared useCounterSetup/useCounter setup for symmetry with the other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/testing/src/testing.spec.ts`:
- Around line 602-627: Replace the inline store in the test with the existing
fixture by adding a doubleIncrementWithSelf action to the useCounterSetup
fixture (or to useCounter if that’s the setup used elsewhere) and then call
useCounterSetup() (or useCounter()) in this test; update the test to reference
the fixture’s doubleIncrementWithSelf and its increment so the behavior and
assertions remain the same while reusing the shared useCounterSetup/useCounter
setup for symmetry with the other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87800265-31b3-428c-9710-450a1aabaee3
📒 Files selected for processing (2)
packages/testing/src/testing.spec.tspackages/testing/src/testing.ts
✅ Files skipped from review due to trivial changes (1)
- packages/testing/src/testing.ts
posva
left a comment
posva
left a comment
There was a problem hiding this comment.
Thanks but these tests are not needed. Let's remove them and instead of adding a NOTE to the jsdocs, add a sub section to https://pinia.vuejs.org/cookbook/testing.html#Customizing-behavior-of-actions that explains this behavior
dennybiasiolli
force-pushed
the
clarify-actions-stubbing
branch
from
40d1e2e to
a197dce
Compare
dennybiasiolli
changed the title
stubActions limitations in Setup stores
|
Ok done, thank you for the feedback |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/docs/cookbook/testing.md`:
- Line 210: The sentence incorrectly suggests using this.increment() for Setup
stores; update the text to remove the this.increment() reference and clarify
that Setup stores should call actions via the store proxy (e.g.,
useStore().increment() or the specific useCounterStore().increment()) so that
stubActions affects them as expected; ensure mentions of increment(),
useStore(), useCounterStore(), Setup store, and stubActions remain consistent
with the example shown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7381756-b62a-40ed-ac51-9bbbab50a87a
📒 Files selected for processing (1)
packages/docs/cookbook/testing.md
Explains that internal action calls in Setup stores bypass stubActions because they use closed-over references instead of the store proxy, and documents the workaround of calling actions via the store proxy.
dennybiasiolli
force-pushed
the
clarify-actions-stubbing
branch
from
a197dce to
9ccf320
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/docs/cookbook/testing.md`:
- Around line 188-191: The doc's description is contradictory: you say
store.incrementTwice() is "stubbed" and yet its internal call to increment()
"will still run"; update the text around useCounterStore(), increment(), and
incrementTwice() to separate the two cases clearly—(a) when incrementTwice()
itself is stubbed its implementation does not run and thus it will not call
increment(), and (b) when only increment() is stubbed via the store proxy,
calling incrementTwice() will run its real implementation and will invoke the
stubbed increment(); reword the example lines to reflect these two distinct
behaviors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40375a97-87d1-455b-8bd6-af79d0194f48
📒 Files selected for processing (1)
packages/docs/cookbook/testing.md
2 participants
Summary
stubActionscannot intercept internal action calls inSetup stores, because they use closed-over references instead of the
store proxy
thisalways routes throughthe proxy, so internal calls are stubbed as expected
useStore()inside the Setupstore to make them interceptable by
stubActionsMotivation
This is a common source of confusion when testing Setup stores — tests
appear to stub an action but internal calls still execute. The existing
docs had no mention of this behaviour or how to work around it.
👤 PR code is human-generated by @dennybiasiolli
🤖 PR text generated with Claude Code
Summary by CodeRabbit