From aedba6d9950f0deef67e96e03a4aa23b5f76a997 Mon Sep 17 00:00:00 2001 From: Sergey Volkov Date: Fri, 23 Jan 2026 14:38:33 +0300 Subject: [PATCH] feat: optimize withViewModel HOC (remove observer wrapper) --- .changeset/tame-lands-drive.md | 5 ++ src/react/hoc/with-view-model.test.tsx | 78 +++++++++++++++++++++++++- src/react/hoc/with-view-model.tsx | 45 ++++++++------- 3 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 .changeset/tame-lands-drive.md diff --git a/.changeset/tame-lands-drive.md b/.changeset/tame-lands-drive.md new file mode 100644 index 00000000..fa93d516 --- /dev/null +++ b/.changeset/tame-lands-drive.md @@ -0,0 +1,5 @@ +--- +"mobx-view-model": minor +--- + +optimize `withViewModel` HOC with replaced `observer()` wrap HOC to `` component use diff --git a/src/react/hoc/with-view-model.test.tsx b/src/react/hoc/with-view-model.test.tsx index 95a20991..b4d47e80 100644 --- a/src/react/hoc/with-view-model.test.tsx +++ b/src/react/hoc/with-view-model.test.tsx @@ -109,6 +109,74 @@ describe('withViewModel', () => { expect(spyFallbackRender).toHaveBeenCalledTimes(1); }); + test('updates UI when isMounted becomes true', async () => { + class VM extends ViewModelBaseMock { + mount() { + setTimeout(() => { + super.mount(); + }, 20); + } + } + const View = ({ model }: ViewModelProps) => { + return
{`hello ${model.id}`}
; + }; + const Fallback = () => { + return
fallback
; + }; + const Component = withViewModel(VM, { + generateId: createIdGenerator(), + fallback: Fallback, + })(View); + + render(); + + expect(screen.getByTestId('fallback')).toBeDefined(); + + await act(async () => { + await sleep(30); + }); + + expect(screen.getByTestId('view')).toBeDefined(); + }); + + test('updates UI when isMounted becomes false', async () => { + class VM extends ViewModelBaseMock { + triggerUnmount() { + this.unmount(); + } + } + const View = ({ model }: ViewModelProps) => { + return ( +
+ +
{`hello ${model.id}`}
+
+ ); + }; + const Fallback = () => { + return
fallback
; + }; + const Component = withViewModel(VM, { + generateId: createIdGenerator(), + fallback: Fallback, + })(View); + + await act(async () => render()); + + expect(screen.getByTestId('view')).toBeDefined(); + + await act(async () => { + fireEvent.click(screen.getByTestId('unmount')); + }); + + expect(screen.getByTestId('fallback')).toBeDefined(); + }); + test('renders fallback before render REAL COMPONENT (times)', async () => { class VM extends ViewModelBaseMock {} const View = ({ model }: ViewModelProps) => { @@ -249,7 +317,7 @@ describe('withViewModel', () => { expect(screen.getByText('second-VM_1')).toBeDefined(); }); - test('withViewModel wrapper should by only mounted (renders 2 times)', () => { + test('withViewModel wrapper should by only mounted (renders 1 time)', () => { class VM extends ViewModelBaseMock {} const View = vi.fn(({ model }: ViewModelProps) => { return
{`hello ${model.id}`}
; @@ -263,7 +331,7 @@ describe('withViewModel', () => { })(View); render(); - expect(useHookSpy).toHaveBeenCalledTimes(2); + expect(useHookSpy).toHaveBeenCalledTimes(1); }); describe('payload manipulations', () => { @@ -746,7 +814,11 @@ describe('withViewModel', () => { await sleep(200); - expect(setPayloadSpy).toHaveBeenCalledTimes(2); + expect(setPayloadSpy).toHaveBeenCalledTimes(1); + expect(setPayloadSpy.mock.calls[0]?.[0]).toMatchObject({ + techreviewId: '1', + selectedCompIds: ['1', '2', '3'], + }); }); }); diff --git a/src/react/hoc/with-view-model.tsx b/src/react/hoc/with-view-model.tsx index a64a69bf..b4435337 100644 --- a/src/react/hoc/with-view-model.tsx +++ b/src/react/hoc/with-view-model.tsx @@ -1,4 +1,4 @@ -import { observer } from 'mobx-react-lite'; +import { Observer, observer } from 'mobx-react-lite'; import { forwardRef, useContext } from 'react'; import type { AnyObject, @@ -373,24 +373,31 @@ const withViewModelWrapper = ( props: componentProps, }) as unknown as AnyViewModel | AnyViewModelSimple; - const isRenderAllowedByStore = - !viewModels || viewModels.isAbleToRenderView(model.id); - - // This condition is works for AnyViewModelSimple too - // All other variants will be bad for performance - const isRenderAllowed = - isRenderAllowedByStore && (model as AnyViewModel).isMounted !== false; - - if (isRenderAllowed) { - return ( - - {Component && } - - ); - } - return ( - FallbackComponent && + + {() => { + const isRenderAllowedByStore = + !viewModels || viewModels.isAbleToRenderView(model.id); + + // This condition is works for AnyViewModelSimple too + // All other variants will be bad for performance + const isRenderAllowed = + isRenderAllowedByStore && + (model as AnyViewModel).isMounted !== false; + + if (isRenderAllowed) { + return ( + + {Component && } + + ); + } + + return FallbackComponent ? ( + + ) : null; + }} + ); }; @@ -400,8 +407,6 @@ const withViewModelWrapper = ( ConnectedViewModel = forwardRef(ConnectedViewModel) as any; } - ConnectedViewModel = observer(ConnectedViewModel); - if (process.env.NODE_ENV !== 'production') { (ConnectedViewModel as React.ComponentType).displayName = `ConnectedViewModel(${VM.name}->Component)`;