[feature] Vue support#60
Conversation
📝 WalkthroughWalkthroughThis PR adds comprehensive Vue 3 integration to the mobx-view-model library, introducing composables for view model lifecycle and lookup, provider components for dependency injection, an observer wrapper for MobX reactivity, detailed documentation, and extensive test coverage alongside configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Vue as Vue Component
participant Observer as observer() HOC
participant Reaction as MobX Reaction
participant Observable as Observable State
Note over Vue,Observable: Initial Render
Vue->>Observer: setup()
Observer->>Reaction: Create & store on proxy
Observer->>Vue: Hook setup/render
Vue->>Vue: Execute render function
Reaction->>Observable: Track observed reads
Note over Vue,Observable: State Change
Observable->>Observable: State mutates
Reaction->>Reaction: Tracked deps changed
Reaction->>Vue: Trigger tracked render
Vue->>Vue: Re-execute render
Note over Vue,Observable: Cleanup
Vue->>Observer: onBeforeUnmount
Observer->>Reaction: dispose()
Reaction-->>Observable: Unsubscribe
sequenceDiagram
participant App as Vue App
participant Provider as ViewModelsProvider
participant Store as ViewModelStore
participant Comp as withViewModel(VM)<br/>Component
participant Observer as observer()
participant VM as ViewModel Instance
App->>Provider: Render with store prop
Provider->>Provider: provide(ViewModelsKey)
Provider->>App: Render slot
App->>Comp: Mount wrapped component
Comp->>Comp: useCreateViewModel(VM)
Comp->>Store: Try lookup by id
alt Found in store
Store-->>VM: Return existing
else Not found
Comp->>VM: Create new instance
Comp->>Store: Mark for attachment
end
Comp->>Observer: Wrap with observer
Observer->>VM: Track observable reads
Comp->>Comp: Render with model prop
Comp->>App: onMounted
App->>Store: Attach view model
Observable->>VM: State changes
Observer->>Observer: Reaction tracks change
Observer->>Comp: Trigger re-render
Comp->>Comp: Re-execute render
sequenceDiagram
participant Child as Child Component
participant Hook as useViewModel(Lookup)
participant Store as useViewModelsStore()
participant Active as useActiveViewModel()
participant Result as ViewModel Instance
Child->>Hook: Call useViewModel(VM)
alt Lookup provided
Hook->>Store: Try store.getViewModel(id)
alt Found
Store-->>Result: Return from store
else Not found
Hook->>Hook: Error: not found in store
end
else No lookup arg
Hook->>Active: useActiveViewModel()
alt Active provided
Active-->>Result: Return from DI
else Not provided
Hook->>Hook: Error: no active VM
end
end
Hook-->>Child: Return resolved ViewModel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 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.
Actionable comments posted: 6
🧹 Nitpick comments (12)
src/vue/use-active-view-model.ts (1)
8-10: Always provide the value (includingnull) to avoid accidental parent fallback.With the guard on line 8, when
provideActiveViewModel(null)is called,provideis never invoked. This allows descendants callinguseActiveViewModel()to inherit from an ancestor provider instead of receiving an explicitnullvalue. In Vue 3,injectresolves to the nearest ancestor provider when an intermediate component doesn't callprovide.Proposed refactor
export const provideActiveViewModel = ( model: AnyViewModel | AnyViewModelSimple | null, ) => { - if (model) { - provide(ActiveViewModelKey, model); - } + provide(ActiveViewModelKey, model); return model; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/use-active-view-model.ts` around lines 8 - 10, The current guard around provide prevents calling provide(ActiveViewModelKey, model) when model is null, letting descendants inject fall back to an ancestor; remove the conditional and always call provide(ActiveViewModelKey, model) (i.e., unconditionally invoke provide inside provideActiveViewModel), and if necessary update the provideActiveViewModel parameter/type to accept null so explicit null is provided and useActiveViewModel() consumers will receive that null rather than an ancestor value.docs/vue/api/observer.md (1)
17-25: Example references undefinedstorevariable.The example uses
store.countbut doesn't show wherestorecomes from. Consider adding the store import or definition to make the example self-contained.📝 Suggested improvement
```ts import { observer } from 'mobx-view-model/vue'; -import { defineComponent } from 'vue'; +import { defineComponent, h } from 'vue'; +import { store } from './store'; // or however the MobX store is accessed export default observer( defineComponent({ name: 'Counter', setup() { return () => h('div', store.count); }, }), ); ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/vue/api/observer.md` around lines 17 - 25, The example uses an undefined store variable; update the example so it's self-contained by importing or defining the MobX store and the Vue h helper where the sample lives. Specifically, add an import or simple inline definition for store (referenced in setup() as store.count) and ensure h is imported from 'vue' alongside defineComponent so observer(defineComponent(...)) compiles and the example shows where store comes from.src/vue/providers.ts (2)
33-36: Same non-reactive pattern applies here.Similar to
ViewModelsProvider, the active view model is provided once at setup. This is fine if the model instance is stable throughout the provider's lifetime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/providers.ts` around lines 33 - 36, The provider currently calls provideActiveViewModel(props.value) once in setup, which doesn't update if the prop changes; modify the setup in this component to make the provided active view model reactive by wrapping props.value in a ref or computed and updating the provided value whenever props.value changes (e.g., call provideActiveViewModel inside a watch/watchEffect on props.value or provide a ref created from props.value), referencing the setup function and provideActiveViewModel to locate and update the logic so consumers see updates when the prop changes.
19-22: Provide pattern does not reactively track prop changes.The
provideViewModelsStore(props.value)is called once during setup. If thevalueprop changes after mount, descendants continue using the original store instance.This is standard behavior for provide/inject and acceptable for stable store contexts. Current tests show stores are always established once per provider. If dynamic store updates become a requirement, consider wrapping the store with
computed()and callingprovide()in a watcher to maintain reactivity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/providers.ts` around lines 19 - 22, The provider currently calls provideViewModelsStore(props.value) only once inside setup, so changes to props.value after mount won't update injected stores; to make it reactive, wrap the incoming prop in a computed (or use toRef) and call provide() inside a watcher that updates the provided value when the computed changes, or alternatively provide a computed-backed wrapper so consumers always read the latest store; locate the setup function and modify the provideViewModelsStore usage (or replace it with provide and a reactive/computed wrapper) to ensure provided value tracks props.value changes.docs/vue/api/use-create-view-model.md (1)
5-5: Minor grammar improvement.The sentence lacks an explicit subject. Consider rephrasing for clarity.
📝 Suggested fix
-`payload` may be a plain value, `ref`, or `computed`; changes update the model via `setPayload`. +The `payload` argument may be a plain value, `ref`, or `computed`; changes update the model via `setPayload`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/vue/api/use-create-view-model.md` at line 5, Rewrite the sentence to include an explicit subject and clearer structure; for example, state that "The payload parameter may be a plain value, a ref, or a computed, and changes to it update the model via setPayload" — update the sentence referencing `payload` and `setPayload` in docs/vue/api/use-create-view-model.md accordingly.src/vue/observer.ts (1)
63-65: Reaction disposal registered twice.The reaction is disposed in two places:
- Via
onBeforeUnmountcallback (lines 63-65)- Via the wrapped
beforeUnmountoption method (lines 95-99)While
disposeReactionhandles the case where the reaction is already disposed (by checkingreaction?.dispose()), this redundancy is intentional but could be confusing. The dual registration ensures cleanup in both Options API and Composition API usage patterns.Consider adding a brief comment explaining why both are needed, or consolidate to one approach if possible.
📝 Suggested comment
+ // Register cleanup for Composition API usage onBeforeUnmount(() => { disposeReaction(proxy); });+ // Also handle cleanup for Options API components that don't use setup's onBeforeUnmount wrapped.beforeUnmount = function () {Also applies to: 95-99
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/observer.ts` around lines 63 - 65, The reaction disposal is registered twice (via onBeforeUnmount(() => disposeReaction(proxy)) and again in the wrapped beforeUnmount option) which is intentional for supporting both Composition API and Options API; either consolidate to a single unified cleanup path (remove one registration and ensure disposeReaction(proxy) is called from the remaining lifecycle hook) or keep both but add a brief clarifying comment next to onBeforeUnmount and the wrapped beforeUnmount (referencing onBeforeUnmount, disposeReaction, and the wrapped beforeUnmount handler) stating that dual registration is deliberate to cover both API usage patterns and that disposeReaction is idempotent so double-calls are safe.src/vue/use-view-model.ts (1)
46-68: Potential fallback toundefinedfordisplayName.If
vmLookupis an object that has neithernamenordisplayName, thedisplayNamevariable remains an empty string from the initial assignment, which is the intended fallback. However, Line 54 casts and accessesdisplayNameunconditionally, which could yieldundefinedif the object lacks that property—though concatenation would coerce it to"undefined".Consider adding a safeguard or a fallback for clarity:
💡 Suggested improvement
if (typeof vmLookup === 'string') { displayName = vmLookup; } else if ('name' in vmLookup) { displayName = vmLookup.name; - } else { - displayName = (vmLookup as AnyObject).displayName; + } else if ('displayName' in vmLookup) { + displayName = (vmLookup as AnyObject).displayName; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/use-view-model.ts` around lines 46 - 68, The code in useViewModel (variable displayName) can end up using an empty string or "undefined" when vmLookup is an object without name or displayName; update the fallback logic so displayName is always a meaningful string (e.g., default to vmLookup.toString() or a literal like '<unknown view model>') before it's interpolated into the thrown Error; locate the displayName assignment logic around vmLookup handling and ensure the final value is normalized (non-empty string) prior to error construction.src/vue/reactivity.test.ts (1)
29-33: Document the rationale for the doublenextTickflush pattern.The
flushhelper usesnextTick→setTimeout→nextTick. While this works, adding a brief comment explaining why this pattern is needed (e.g., waiting for both Vue's and MobX's async update cycles) would help future maintainers.📝 Proposed documentation
const flush = async () => { + // Wait for Vue's reactivity queue, then yield to the event loop for MobX + // reaction scheduling, then wait for Vue again to process any triggered updates. await nextTick(); await new Promise((resolve) => setTimeout(resolve, 0)); await nextTick(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/reactivity.test.ts` around lines 29 - 33, The test helper flush uses a double nextTick with an intervening setTimeout (function flush) but lacks an explanatory comment; add a concise inline comment above the flush definition that explains why the pattern is required (e.g., to wait for Vue's microtask queue, then yield to the macrotask loop to allow MobX or other async updates to propagate, then await Vue's nextTick again to settle final DOM/reactive updates). Reference the helper name flush and the calls to nextTick and setTimeout in the comment so future maintainers understand the rationale and ordering of async update cycles.src/vue/with-view-model.ts (3)
206-211: Non-reactivepropsin config may cause stale data.
componentProps.valueis accessed once during setup and passed touseCreateViewModel. If the component receives new props after initial render, thepropsfield in the view model's config will contain stale values.If this is intentional (props are only used for initial creation), consider documenting this behavior. Otherwise, consider whether the view model needs a way to receive prop updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/with-view-model.ts` around lines 206 - 211, The code passes a non-reactive snapshot componentProps.value into useCreateViewModel which can lead to stale props in the created model; change the call so the view model receives a reactive source (e.g., pass componentProps or a computed/ref rather than componentProps.value) or add a watcher to propagate prop updates into the model after creation; update the useCreateViewModel invocation (and any related VM/ConnectedViewModel initialization) to accept and use a reactive props ref or implement a prop-update hook so the view model always sees current prop values.
250-255: Mutating sharedanchorsarray could cause unexpected behavior.The
connect()method pushes to theanchorsarray in-place. Sinceconfig.anchorsis assigned toanchors(line 243), andconfigmay be shared or reused, this mutation could affect other usages of the same config object.Consider creating a copy of the anchors array to avoid unexpected side effects.
🔧 Proposed fix
config.component = ConnectedViewModel as any; - config.anchors ??= []; - - const anchors = config.anchors; + const anchors = [...(config.anchors ?? [])]; const ConnectedWithConnect =🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/with-view-model.ts` around lines 250 - 255, The connect method mutates the shared anchors array; update ConnectedWithConnect.connect to avoid in-place mutation by creating a new array when adding an anchor (do not push directly into anchors). Locate ConnectedWithConnect.connect and the shared variable anchors (and where config.anchors is assigned) and replace the push with logic that constructs a new array containing existing anchors plus the new anchor (and, if needed, reassigns that new array back to the shared reference such as config.anchors) so other consumers of the original config aren't mutated.
139-151: Heuristic for distinguishing config from component may be fragile.The check
typeof configOrComponent === 'object' && !configOrComponent?.setupattempts to distinguish between a config object and a Vue component. However, this could incorrectly identify functional components (which may lacksetup) or config objects that accidentally have asetupproperty.Consider using a more explicit marker or checking for Vue component characteristics like
render,template, or using Vue'sisVNodeutilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/with-view-model.ts` around lines 139 - 151, The heuristic using typeof configOrComponent === 'object' && !configOrComponent?.setup is fragile; change the detection to explicitly identify config objects versus Vue components by either (A) requiring and checking for an explicit marker on config (e.g., config.__isViewModelConfig === true) and treating anything else as a Component, or (B) detect components by Vue-like characteristics (e.g., treat as Component when typeof configOrComponent === 'function' OR 'render' in configOrComponent OR 'template' in configOrComponent OR 'setup' in configOrComponent) and otherwise treat as config; update the branch that builds finalConfig and the return of withViewModelWrapper(VM, finalConfig, Component) accordingly so configOrComponent is unambiguously interpreted.src/vue/vue.test.ts (1)
9-20: Consider extracting themounthelper to a shared test utility.The
mounthelper is duplicated across multiple test files (vue.test.ts,reactivity.test.ts,observer.test.ts,with-view-model.test.ts). Consider extracting it to a shared test utility file to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vue/vue.test.ts` around lines 9 - 20, Duplicate mount helper found in multiple test files; extract it into a shared test utility module. Create a new test utility file exporting the mount function (using the existing logic that creates an element, app via createApp(root), mounts it, and returns unmount) and update tests that use mount (references: mount, defineComponent, createApp) to import it from the new module; ensure teardown behavior (app.unmount and el.remove) remains unchanged and update imports in vue.test.ts, reactivity.test.ts, observer.test.ts, and with-view-model.test.ts to use the shared helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/vue/only-view-model.ts`:
- Around line 36-50: The render reads the MobX observable vm.isMounted inside
setup but the component isn't wrapped with observer, so changes to isMounted
won't trigger re-renders; wrap the component with observer (i.e., export the
component definition through observer) so the setup/render function that uses vm
and checks vm.isMounted becomes reactive, or alternatively convert vm.isMounted
into a Vue ref and track that inside setup; locate the setup function that calls
useCreateViewModel and the vm variable (and the return render that uses
slots.default and vm.isMounted) and apply observer wrapping to the component
definition (or expose a reactive ref that mirrors vm.isMounted and use that in
the render).
In `@src/vue/types.ts`:
- Line 11: Update the factory signature to use the proper typed config instead
of any: replace the parameter type in the factory?: (config: any) => TViewModel
declaration with the existing ViewModelCreateConfig<TViewModel> (or reuse
CreateViewModelFactoryFn<TViewModel> / ViewModelsConfig<TViewModel>['factory']
as React does) so the factory matches the typed CreateViewModelFactoryFn and
returns TViewModel while accepting ViewModelCreateConfig<TViewModel>.
In `@src/vue/use-create-view-model.ts`:
- Around line 180-186: The onMounted callback in useCreateViewModelSimple calls
viewModels.attach(instance) without the void operator, creating an unhandled
promise; update the onMounted handler in useCreateViewModelSimple to use void
viewModels.attach(instance) (keeping the existing else branch that calls
instance.mount?.()) so the attach call is intentionally fire-and-forget and
consistent with useCreateViewModelBase.
- Around line 170-178: The bug is that useCreateViewModelSimple passes the Ref
object (payloadRef) into instance.setPayload, violating the
ViewModelSimple.setPayload(payload: Payload) contract and causing inconsistency
with useCreateViewModelBase. Fix by passing the unwrapped value (use
unref(payloadRef) or payloadRef.value) to instance.setPayload inside the watch
callback in useCreateViewModelSimple so setPayload receives a Payload, matching
useCreateViewModelBase and the ViewModelSimple signature.
In `@src/vue/use-view-model-store.ts`:
- Around line 10-11: The function useViewModelsStore currently forces a non-null
ViewModelStore by double-casting null; change its signature/return to allow null
(ViewModelStore | null) and return the injected value as nullable instead of
casting — update useViewModelsStore to return inject(ViewModelsKey) typed as
ViewModelStore | null (mirror the nullable pattern used in useActiveViewModel)
so call sites can continue to use optional chaining safely.
In `@tsconfig.json`:
- Around line 26-27: The package.json is missing an exports field so consumers
cannot reliably import the "mobx-view-model/vue" subpath; add an "exports" map
to package.json that declares the main package entry (".") and the "./vue"
subpath pointing to the compiled Vue entry (the file that corresponds to your
tsconfig alias "./src/vue/index.ts"), and include corresponding types paths
(e.g., "./types" or "./vue": "./dist/vue/index.d.ts") if you ship type
declarations; ensure the export targets match your build output filenames so
imports of "mobx-view-model/vue" resolve for published consumers.
---
Nitpick comments:
In `@docs/vue/api/observer.md`:
- Around line 17-25: The example uses an undefined store variable; update the
example so it's self-contained by importing or defining the MobX store and the
Vue h helper where the sample lives. Specifically, add an import or simple
inline definition for store (referenced in setup() as store.count) and ensure h
is imported from 'vue' alongside defineComponent so
observer(defineComponent(...)) compiles and the example shows where store comes
from.
In `@docs/vue/api/use-create-view-model.md`:
- Line 5: Rewrite the sentence to include an explicit subject and clearer
structure; for example, state that "The payload parameter may be a plain value,
a ref, or a computed, and changes to it update the model via setPayload" —
update the sentence referencing `payload` and `setPayload` in
docs/vue/api/use-create-view-model.md accordingly.
In `@src/vue/observer.ts`:
- Around line 63-65: The reaction disposal is registered twice (via
onBeforeUnmount(() => disposeReaction(proxy)) and again in the wrapped
beforeUnmount option) which is intentional for supporting both Composition API
and Options API; either consolidate to a single unified cleanup path (remove one
registration and ensure disposeReaction(proxy) is called from the remaining
lifecycle hook) or keep both but add a brief clarifying comment next to
onBeforeUnmount and the wrapped beforeUnmount (referencing onBeforeUnmount,
disposeReaction, and the wrapped beforeUnmount handler) stating that dual
registration is deliberate to cover both API usage patterns and that
disposeReaction is idempotent so double-calls are safe.
In `@src/vue/providers.ts`:
- Around line 33-36: The provider currently calls
provideActiveViewModel(props.value) once in setup, which doesn't update if the
prop changes; modify the setup in this component to make the provided active
view model reactive by wrapping props.value in a ref or computed and updating
the provided value whenever props.value changes (e.g., call
provideActiveViewModel inside a watch/watchEffect on props.value or provide a
ref created from props.value), referencing the setup function and
provideActiveViewModel to locate and update the logic so consumers see updates
when the prop changes.
- Around line 19-22: The provider currently calls
provideViewModelsStore(props.value) only once inside setup, so changes to
props.value after mount won't update injected stores; to make it reactive, wrap
the incoming prop in a computed (or use toRef) and call provide() inside a
watcher that updates the provided value when the computed changes, or
alternatively provide a computed-backed wrapper so consumers always read the
latest store; locate the setup function and modify the provideViewModelsStore
usage (or replace it with provide and a reactive/computed wrapper) to ensure
provided value tracks props.value changes.
In `@src/vue/reactivity.test.ts`:
- Around line 29-33: The test helper flush uses a double nextTick with an
intervening setTimeout (function flush) but lacks an explanatory comment; add a
concise inline comment above the flush definition that explains why the pattern
is required (e.g., to wait for Vue's microtask queue, then yield to the
macrotask loop to allow MobX or other async updates to propagate, then await
Vue's nextTick again to settle final DOM/reactive updates). Reference the helper
name flush and the calls to nextTick and setTimeout in the comment so future
maintainers understand the rationale and ordering of async update cycles.
In `@src/vue/use-active-view-model.ts`:
- Around line 8-10: The current guard around provide prevents calling
provide(ActiveViewModelKey, model) when model is null, letting descendants
inject fall back to an ancestor; remove the conditional and always call
provide(ActiveViewModelKey, model) (i.e., unconditionally invoke provide inside
provideActiveViewModel), and if necessary update the provideActiveViewModel
parameter/type to accept null so explicit null is provided and
useActiveViewModel() consumers will receive that null rather than an ancestor
value.
In `@src/vue/use-view-model.ts`:
- Around line 46-68: The code in useViewModel (variable displayName) can end up
using an empty string or "undefined" when vmLookup is an object without name or
displayName; update the fallback logic so displayName is always a meaningful
string (e.g., default to vmLookup.toString() or a literal like '<unknown view
model>') before it's interpolated into the thrown Error; locate the displayName
assignment logic around vmLookup handling and ensure the final value is
normalized (non-empty string) prior to error construction.
In `@src/vue/vue.test.ts`:
- Around line 9-20: Duplicate mount helper found in multiple test files; extract
it into a shared test utility module. Create a new test utility file exporting
the mount function (using the existing logic that creates an element, app via
createApp(root), mounts it, and returns unmount) and update tests that use mount
(references: mount, defineComponent, createApp) to import it from the new
module; ensure teardown behavior (app.unmount and el.remove) remains unchanged
and update imports in vue.test.ts, reactivity.test.ts, observer.test.ts, and
with-view-model.test.ts to use the shared helper.
In `@src/vue/with-view-model.ts`:
- Around line 206-211: The code passes a non-reactive snapshot
componentProps.value into useCreateViewModel which can lead to stale props in
the created model; change the call so the view model receives a reactive source
(e.g., pass componentProps or a computed/ref rather than componentProps.value)
or add a watcher to propagate prop updates into the model after creation; update
the useCreateViewModel invocation (and any related VM/ConnectedViewModel
initialization) to accept and use a reactive props ref or implement a
prop-update hook so the view model always sees current prop values.
- Around line 250-255: The connect method mutates the shared anchors array;
update ConnectedWithConnect.connect to avoid in-place mutation by creating a new
array when adding an anchor (do not push directly into anchors). Locate
ConnectedWithConnect.connect and the shared variable anchors (and where
config.anchors is assigned) and replace the push with logic that constructs a
new array containing existing anchors plus the new anchor (and, if needed,
reassigns that new array back to the shared reference such as config.anchors) so
other consumers of the original config aren't mutated.
- Around line 139-151: The heuristic using typeof configOrComponent === 'object'
&& !configOrComponent?.setup is fragile; change the detection to explicitly
identify config objects versus Vue components by either (A) requiring and
checking for an explicit marker on config (e.g., config.__isViewModelConfig ===
true) and treating anything else as a Component, or (B) detect components by
Vue-like characteristics (e.g., treat as Component when typeof configOrComponent
=== 'function' OR 'render' in configOrComponent OR 'template' in
configOrComponent OR 'setup' in configOrComponent) and otherwise treat as
config; update the branch that builds finalConfig and the return of
withViewModelWrapper(VM, finalConfig, Component) accordingly so
configOrComponent is unambiguously interpreted.
🪄 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: a1f1d680-152a-4ff8-8105-884eb15ec16f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
docs/.vitepress/config.mtsdocs/vue/api/observer.mddocs/vue/api/only-view-model.mddocs/vue/api/use-create-view-model.mddocs/vue/api/use-view-model.mddocs/vue/api/view-models-provider.mddocs/vue/api/with-view-model.mddocs/vue/integration.mddocs/vue/ssr.mdpackage.jsonsrc/index.tssrc/react/hoc/with-view-model.test.fixture.tssrc/react/hoc/with-view-model.test.tsxsrc/view-model/view-model-simple.test.tssrc/view-model/view-model.base.tssrc/vue/index.tssrc/vue/injection-keys.tssrc/vue/observer.test.tssrc/vue/observer.tssrc/vue/only-view-model.tssrc/vue/providers.tssrc/vue/reactivity.test.tssrc/vue/types.tssrc/vue/use-active-view-model.tssrc/vue/use-create-view-model.tssrc/vue/use-view-model-store.tssrc/vue/use-view-model.tssrc/vue/vue.test.tssrc/vue/with-view-model.test.tssrc/vue/with-view-model.tstsconfig.jsontsconfig.test.json
| setup(props, { slots }) { | ||
| const vm = useCreateViewModel( | ||
| props.model as unknown as Class<AnyViewModel | ViewModelSimple>, | ||
| props.payload, | ||
| props.config, | ||
| ) as AnyViewModel | ViewModelSimple; | ||
|
|
||
| return () => { | ||
| if ((vm as AnyViewModel).isMounted === false) { | ||
| return null; | ||
| } | ||
|
|
||
| return slots.default?.({ model: vm }); | ||
| }; | ||
| }, |
There was a problem hiding this comment.
Missing MobX reactivity tracking for isMounted check.
The render function reads vm.isMounted (a MobX observable) but this component is not wrapped with observer(). When isMounted changes from false to true, Vue won't be notified to re-render, potentially leaving the component stuck showing null.
Consider wrapping the component with observer() or using a Vue reactive ref to track mount state.
🐛 Proposed fix to wrap with observer
+import { observer } from './observer.js';
+
export type OnlyViewModelProps<TViewModel extends AnyViewModel> = {
// ...
};
-export const OnlyViewModel = defineComponent({
+export const OnlyViewModel = observer(defineComponent({
name: 'OnlyViewModel',
// ...
-});
+}));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vue/only-view-model.ts` around lines 36 - 50, The render reads the MobX
observable vm.isMounted inside setup but the component isn't wrapped with
observer, so changes to isMounted won't trigger re-renders; wrap the component
with observer (i.e., export the component definition through observer) so the
setup/render function that uses vm and checks vm.isMounted becomes reactive, or
alternatively convert vm.isMounted into a Vue ref and track that inside setup;
locate the setup function that calls useCreateViewModel and the vm variable (and
the return render that uses slots.default and vm.isMounted) and apply observer
wrapping to the component definition (or expose a reactive ref that mirrors
vm.isMounted and use that in the render).
| export interface UseCreateViewModelConfig<TViewModel extends AnyViewModel> { | ||
| id?: Maybe<string>; | ||
| generateId?: (ctx: AnyObject) => string; | ||
| factory?: (config: any) => TViewModel; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify what create-config type is available to reuse in UseCreateViewModelConfig.factory
rg -n --type=ts "type ViewModelCreateConfig|interface ViewModelCreateConfig|UseCreateViewModelConfig|factory\\?:" srcRepository: js2me/mobx-view-model
Length of output: 2290
🏁 Script executed:
cat -n src/vue/types.tsRepository: js2me/mobx-view-model
Length of output: 768
🏁 Script executed:
cat -n src/view-model/view-model.store.types.ts | head -40Repository: js2me/mobx-view-model
Length of output: 1588
🏁 Script executed:
cat -n src/react/hooks/use-create-view-model.ts | head -50Repository: js2me/mobx-view-model
Length of output: 2050
🏁 Script executed:
cat -n src/config/types.ts | grep -A 5 -B 5 "factory"Repository: js2me/mobx-view-model
Length of output: 1728
🏁 Script executed:
cat -n src/config/types.ts | grep -B 10 "CreateViewModelFactoryFn" | head -30Repository: js2me/mobx-view-model
Length of output: 1464
🏁 Script executed:
rg -n "CreateViewModelFactoryFn" src --type=ts -A 2 -B 2Repository: js2me/mobx-view-model
Length of output: 878
🏁 Script executed:
cat -n src/vue/types.tsRepository: js2me/mobx-view-model
Length of output: 768
🏁 Script executed:
cat -n src/config/types.ts | head -15Repository: js2me/mobx-view-model
Length of output: 736
🏁 Script executed:
rg -n "export.*ViewModelCreateConfig" src --type=tsRepository: js2me/mobx-view-model
Length of output: 172
🏁 Script executed:
cat -n src/view-model/index.ts | grep -i "viewmodelcreateconfig"Repository: js2me/mobx-view-model
Length of output: 47
🏁 Script executed:
cat -n src/config/index.ts | grep -i "viewmodelsconfigg\|export"Repository: js2me/mobx-view-model
Length of output: 246
🏁 Script executed:
cat -n src/view-model/index.ts | head -20Repository: js2me/mobx-view-model
Length of output: 442
🏁 Script executed:
rg -n "export" src/config/types.ts | head -20Repository: js2me/mobx-view-model
Length of output: 294
🏁 Script executed:
rg -n "CreateViewModelFactoryFn" src/view-model --type=tsRepository: js2me/mobx-view-model
Length of output: 47
🏁 Script executed:
rg -n "export.*CreateViewModelFactoryFn" src --type=tsRepository: js2me/mobx-view-model
Length of output: 125
Replace any with properly typed config in the factory callback.
Line 11 uses any for the config parameter, weakening type safety. The proper type is ViewModelCreateConfig<TViewModel>, which is already used in React's equivalent implementation. Update the factory type signature to match the CreateViewModelFactoryFn<TViewModel> pattern defined in the config module, or reference it via ViewModelsConfig<TViewModel>['factory'] like React does.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vue/types.ts` at line 11, Update the factory signature to use the proper
typed config instead of any: replace the parameter type in the factory?:
(config: any) => TViewModel declaration with the existing
ViewModelCreateConfig<TViewModel> (or reuse CreateViewModelFactoryFn<TViewModel>
/ ViewModelsConfig<TViewModel>['factory'] as React does) so the factory matches
the typed CreateViewModelFactoryFn and returns TViewModel while accepting
ViewModelCreateConfig<TViewModel>.
| if ('setPayload' in instance) { | ||
| watch( | ||
| payloadRef, | ||
| () => { | ||
| instance.setPayload?.(payloadRef); | ||
| }, | ||
| { immediate: true }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether ViewModelSimple.setPayload is documented to receive refs
rg -n -A3 'setPayload' docs/vue/
# Check ViewModelSimple type definition for setPayload signature
ast-grep --pattern $'interface ViewModelSimple {
$$$
setPayload($_): $_;
$$$
}'Repository: js2me/mobx-view-model
Length of output: 340
🏁 Script executed:
# Check the structure of the file and find the two functions
wc -l src/vue/use-create-view-model.tsRepository: js2me/mobx-view-model
Length of output: 101
🏁 Script executed:
# Look at lines 134-150 (useCreateViewModelBase)
sed -n '130,155p' src/vue/use-create-view-model.tsRepository: js2me/mobx-view-model
Length of output: 499
🏁 Script executed:
# Look at lines 170-178 (useCreateViewModelSimple)
sed -n '165,185p' src/vue/use-create-view-model.tsRepository: js2me/mobx-view-model
Length of output: 499
🏁 Script executed:
# Check ViewModelBase and ViewModelSimple type definitions
rg -n 'interface ViewModelBase|interface ViewModelSimple' src/Repository: js2me/mobx-view-model
Length of output: 303
🏁 Script executed:
# Check the test file at lines 43-44
sed -n '40,50p' src/vue/vue.test.tsRepository: js2me/mobx-view-model
Length of output: 415
🏁 Script executed:
# Check ViewModelBase interface for setPayload
sed -n '1,50p' src/view-model/view-model-base.ts | head -30
rg -n 'setPayload' src/view-model/view-model-base.ts -B2 -A2Repository: js2me/mobx-view-model
Length of output: 215
🏁 Script executed:
# Check ViewModelSimple interface for setPayload
sed -n '1,50p' src/view-model/view-model-simple.ts | head -30
rg -n 'setPayload' src/view-model/view-model-simple.ts -B2 -A2Repository: js2me/mobx-view-model
Length of output: 1104
🏁 Script executed:
# Check if there's any design doc or README explaining this difference
rg -n 'setPayload' README.md docs/ -B2 -A2Repository: js2me/mobx-view-model
Length of output: 2135
🏁 Script executed:
# Check the exact context around setPayload call in useCreateViewModelSimple
sed -n '165,185p' src/vue/use-create-view-model.tsRepository: js2me/mobx-view-model
Length of output: 499
🏁 Script executed:
# Look for type casting or assertions
rg -n 'setPayload' src/vue/use-create-view-model.ts -B3 -A3Repository: js2me/mobx-view-model
Length of output: 661
🏁 Script executed:
# Check if payloadRef is typed as Ref
sed -n '150,175p' src/vue/use-create-view-model.tsRepository: js2me/mobx-view-model
Length of output: 652
setPayload receives a Ref object instead of the unwrapped value, violating the type contract.
The setPayload(payload: Payload) signature in ViewModelSimple expects an unwrapped value, but useCreateViewModelSimple passes payloadRef (a Ref object) at line 174. This contradicts useCreateViewModelBase, which correctly passes unref(payload) at line 138.
The test in vue.test.ts (lines 43–44) confirms this is intentional behavior, using as any type assertions to suppress type checking when accessing the ref's .value property. This creates an undocumented API inconsistency where ViewModelBase.setPayload receives values while ViewModelSimple.setPayload receives refs, despite both implementing the same setPayload(payload: Payload) interface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vue/use-create-view-model.ts` around lines 170 - 178, The bug is that
useCreateViewModelSimple passes the Ref object (payloadRef) into
instance.setPayload, violating the ViewModelSimple.setPayload(payload: Payload)
contract and causing inconsistency with useCreateViewModelBase. Fix by passing
the unwrapped value (use unref(payloadRef) or payloadRef.value) to
instance.setPayload inside the watch callback in useCreateViewModelSimple so
setPayload receives a Payload, matching useCreateViewModelBase and the
ViewModelSimple signature.
| onMounted(() => { | ||
| if (viewModels) { | ||
| viewModels.attach(instance); | ||
| } else { | ||
| instance.mount?.(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Inconsistent handling of async attach in simple VM path.
In useCreateViewModelBase (line 120), viewModels.attach(instance) is properly marked with void to indicate intentional fire-and-forget. However, in useCreateViewModelSimple (line 182), the same call lacks the void operator, creating an unhandled promise.
🔧 Proposed fix for consistency
onMounted(() => {
if (viewModels) {
- viewModels.attach(instance);
+ void viewModels.attach(instance);
} else {
instance.mount?.();
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vue/use-create-view-model.ts` around lines 180 - 186, The onMounted
callback in useCreateViewModelSimple calls viewModels.attach(instance) without
the void operator, creating an unhandled promise; update the onMounted handler
in useCreateViewModelSimple to use void viewModels.attach(instance) (keeping the
existing else branch that calls instance.mount?.()) so the attach call is
intentionally fire-and-forget and consistent with useCreateViewModelBase.
| export const useViewModelsStore = () => | ||
| inject(ViewModelsKey, null as unknown as ViewModelStore); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vue 3 with TypeScript, what is the recommended typing pattern for injectwhen the provider may be absent and default isnull?
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🏁 Script executed:
# First, let's examine the file in question
cat -n src/vue/use-view-model-store.tsRepository: js2me/mobx-view-model
Length of output: 516
🏁 Script executed:
# Search for ViewModelsKey and ViewModelStore definitions
rg "ViewModelsKey|ViewModelStore" -B 2 -A 2Repository: js2me/mobx-view-model
Length of output: 50379
🏁 Script executed:
# Look for usages of useViewModelsStore to understand the impact
rg "useViewModelsStore" -B 2 -A 2Repository: js2me/mobx-view-model
Length of output: 2773
🏁 Script executed:
# Search for other inject() usage patterns in the codebase
rg "inject\(" -B 1 -A 1 | head -100Repository: js2me/mobx-view-model
Length of output: 417
Change return type to ViewModelStore | null for consistency with similar injection patterns.
The double type-cast (null as unknown as ViewModelStore) circumvents TypeScript's type system. The codebase already demonstrates the correct pattern in useActiveViewModel which returns AnyViewModel | AnyViewModelSimple | null. All call sites already handle null implicitly via optional chaining (viewModels?.get()), confirming the intent to make this nullable.
Proposed refactor
export const useViewModelsStore = () =>
- inject(ViewModelsKey, null as unknown as ViewModelStore);
+ inject(ViewModelsKey, null as ViewModelStore | null);📝 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.
| export const useViewModelsStore = () => | |
| inject(ViewModelsKey, null as unknown as ViewModelStore); | |
| export const useViewModelsStore = () => | |
| inject(ViewModelsKey, null as ViewModelStore | null); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/vue/use-view-model-store.ts` around lines 10 - 11, The function
useViewModelsStore currently forces a non-null ViewModelStore by double-casting
null; change its signature/return to allow null (ViewModelStore | null) and
return the injected value as nullable instead of casting — update
useViewModelsStore to return inject(ViewModelsKey) typed as ViewModelStore |
null (mirror the nullable pattern used in useActiveViewModel) so call sites can
continue to use optional chaining safely.
| "mobx-view-model": ["./src/index.ts"], | ||
| "mobx-view-model/vue": ["./src/vue/index.ts"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether published-entry metadata includes the Vue subpath
jq '{name, type, main, module, types, exports, typesVersions}' package.json
# Check where the subpath is referenced in source/docs
rg -n "mobx-view-model/vue|./vue/index" .Repository: js2me/mobx-view-model
Length of output: 2168
🏁 Script executed:
cat package.jsonRepository: js2me/mobx-view-model
Length of output: 3142
🏁 Script executed:
cat vite.config.tsRepository: js2me/mobx-view-model
Length of output: 228
🏁 Script executed:
find src -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: js2me/mobx-view-model
Length of output: 755
🏁 Script executed:
find src -type f \( -name "*.ts" -o -name "*.tsx" \) | grep -i vueRepository: js2me/mobx-view-model
Length of output: 447
🏁 Script executed:
# Check if there's any build output or dist structure
ls -la dist/ 2>/dev/null | head -20 || echo "No dist directory"
# Check for any .d.ts files indicating TypeScript declarations
find . -name "package.json" -type f | grep -v node_modules | head -10Repository: js2me/mobx-view-model
Length of output: 99
Add exports field to package.json to declare the Vue subpath for published consumers.
The TypeScript path alias on lines 26-27 of tsconfig.json only affects this repo's local compilation. For consumers to reliably import mobx-view-model/vue, the subpath must be declared in package.json's exports field. Currently, package.json lacks an exports field entirely, which means subpath import reliability depends on the resolver/tooling used by consumers and can break unexpectedly.
Documentation extensively references the mobx-view-model/vue subpath import pattern across multiple files, so this needs to be properly declared in package metadata for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` around lines 26 - 27, The package.json is missing an exports
field so consumers cannot reliably import the "mobx-view-model/vue" subpath; add
an "exports" map to package.json that declares the main package entry (".") and
the "./vue" subpath pointing to the compiled Vue entry (the file that
corresponds to your tsconfig alias "./src/vue/index.ts"), and include
corresponding types paths (e.g., "./types" or "./vue": "./dist/vue/index.d.ts")
if you ship type declarations; ensure the export targets match your build output
filenames so imports of "mobx-view-model/vue" resolve for published consumers.
1 participant
Summary by CodeRabbit
Vue 3 Support
New Features
useCreateViewModel,useViewModel), and provider components for view model managementDocumentation
Chores