Skip to content

Commit cba8358

Browse files
committed
fix: try to improve withViewModel HOC, block render view if VM is not mounted
1 parent 6e9bc06 commit cba8358

File tree

5 files changed

+67
-19
lines changed

5 files changed

+67
-19
lines changed

src/hoc/with-view-model.test.tsx

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import { observer } from 'mobx-react-lite';
33
import { PropsWithChildren, ReactNode, useState } from 'react';
44
import { describe, expect, test, vi } from 'vitest';
55

6-
import { ViewModelStore, ViewModelStoreImpl, ViewModelsProvider } from '..';
6+
import { ViewModelStore, ViewModelsProvider } from '..';
77
import { createCounter } from '../utils';
88
import { EmptyObject } from '../utils/types';
99
import { ViewModelMock } from '../view-model/view-model.impl.test';
10+
import { ViewModelStoreMock } from '../view-model/view-model.store.impl.test';
1011

1112
import { ViewModelProps, withViewModel } from './with-view-model';
1213

@@ -223,14 +224,16 @@ describe('withViewModel', () => {
223224
expect(vm?.payload).toEqual({ counter: 3 });
224225
});
225226

226-
test('access to parent view model x3', async ({ task }) => {
227+
test('access to parent view model x3', async ({ task, expect }) => {
227228
class VM1 extends ViewModelMock {
228229
vm1Value = 'foo';
229230
}
230231
const Component1 = withViewModel(VM1)(({
231232
children,
232233
model,
233234
}: PropsWithChildren & ViewModelProps<VM1>) => {
235+
expect(model.isMounted).toBeTruthy();
236+
expect(model.spies.mount).toBeCalledTimes(1);
234237
return <div data-testid={`vm-${model.vm1Value}`}>{children}</div>;
235238
});
236239

@@ -241,6 +244,8 @@ describe('withViewModel', () => {
241244
children,
242245
model,
243246
}: PropsWithChildren & ViewModelProps<VM2>) => {
247+
expect(model.isMounted).toBeTruthy();
248+
expect(model.spies.mount).toBeCalledTimes(1);
244249
return (
245250
<div
246251
data-testid={`vm-${model.vm2Value}-${model.parentViewModel.vm1Value}`}
@@ -257,6 +262,8 @@ describe('withViewModel', () => {
257262
children,
258263
model,
259264
}: PropsWithChildren & ViewModelProps<VM3>) => {
265+
expect(model.isMounted).toBeTruthy();
266+
expect(model.spies.mount).toBeCalledTimes(1);
260267
return (
261268
<div
262269
data-testid={`vm-${model.vm3Value}-${model.parentViewModel.vm2Value}`}
@@ -292,7 +299,7 @@ describe('withViewModel', () => {
292299
);
293300
});
294301
const Component = withViewModel(VM, { generateId: () => '1' })(View);
295-
const vmStore = new ViewModelStoreImpl();
302+
const vmStore = new ViewModelStoreMock();
296303

297304
const Wrapper = ({ children }: { children?: ReactNode }) => {
298305
return (
@@ -328,7 +335,7 @@ describe('withViewModel', () => {
328335
);
329336
});
330337
const Component = withViewModel(VM, { generateId: () => '1' })(View);
331-
const vmStore = new ViewModelStoreImpl();
338+
const vmStore = new ViewModelStoreMock();
332339

333340
const Wrapper = ({ children }: { children?: ReactNode }) => {
334341
return (
@@ -343,10 +350,15 @@ describe('withViewModel', () => {
343350
);
344351

345352
expect(viewModels).toBeDefined();
353+
expect(vmStore.spies.get).toHaveBeenCalledTimes(0);
354+
expect(vmStore._instanceAttachedCount.size).toBe(1);
355+
expect(vmStore._unmountingViews.size).toBe(0);
356+
expect(vmStore.mountedViewsCount).toBe(1);
357+
expect(vmStore._mountingViews.size).toBe(0);
346358
});
347359

348360
test('access to parent view model x3', async ({ task }) => {
349-
const vmStore = new ViewModelStoreImpl();
361+
const vmStore = new ViewModelStoreMock();
350362

351363
const Wrapper = ({ children }: { children?: ReactNode }) => {
352364
return (
@@ -412,6 +424,11 @@ describe('withViewModel', () => {
412424
expect(container.firstChild).toMatchFileSnapshot(
413425
`../../tests/snapshots/hoc/with-view-model/view-model-store/${task.name}.html`,
414426
);
427+
expect(vmStore.spies.get).toHaveBeenCalledTimes(0);
428+
expect(vmStore._instanceAttachedCount.size).toBe(3);
429+
expect(vmStore._unmountingViews.size).toBe(0);
430+
expect(vmStore.mountedViewsCount).toBe(3);
431+
expect(vmStore._mountingViews.size).toBe(0);
415432
});
416433
});
417434
});

src/hoc/with-view-model.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,17 @@ export function withViewModel(
137137
new Model(configCreate);
138138

139139
instances.set(id, instance);
140-
}
141-
142-
const instance: ViewModel = instances.get(id)!;
143140

144-
useEffect(() => {
145141
if (viewModels) {
146142
viewModels.attach(instance);
147143
} else {
148144
instance.mount();
149145
}
146+
}
147+
148+
const instance: ViewModel = instances.get(id)!;
150149

150+
useEffect(() => {
151151
return () => {
152152
if (viewModels) {
153153
viewModels.detach(id);
@@ -166,7 +166,10 @@ export function withViewModel(
166166

167167
config?.reactHooks?.(allProps);
168168

169-
if ((!viewModels || viewModels.isAbleToRenderView(id)) && instance) {
169+
if (
170+
(!viewModels || viewModels.isAbleToRenderView(id)) &&
171+
instance.isMounted
172+
) {
170173
return (
171174
<ActiveViewModelContext.Provider value={instance}>
172175
<Component {...(componentProps as any)} model={instance} />

src/hooks/use-view-model.test.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ import { act, fireEvent, render, screen } from '@testing-library/react';
22
import { ReactNode, useState } from 'react';
33
import { beforeEach, describe, expect, test } from 'vitest';
44

5-
import { ViewModelStore, ViewModelStoreImpl, ViewModelsProvider } from '..';
5+
import { ViewModelStore, ViewModelsProvider } from '..';
66
import { withViewModel } from '../hoc';
77
import { ViewModelMock } from '../view-model/view-model.impl.test';
8+
import { ViewModelStoreMock } from '../view-model/view-model.store.impl.test';
89

910
import { useViewModel } from './use-view-model';
1011

@@ -118,7 +119,7 @@ describe('withViewModel', () => {
118119
});
119120

120121
test(`renders (${depth} depth)`, async () => {
121-
const vmStore = new ViewModelStoreImpl();
122+
const vmStore = new ViewModelStoreMock();
122123

123124
const WrappedDepthComponent = () => {
124125
const reversed = [...depthsComponents].reverse();
@@ -220,7 +221,7 @@ describe('withViewModel', () => {
220221
test('container renders VM with fixed id and some child with dynamic id', async ({
221222
task,
222223
}) => {
223-
const vmStore = new ViewModelStoreImpl();
224+
const vmStore = new ViewModelStoreMock();
224225

225226
class LayoutVM extends ViewModelMock {}
226227

@@ -257,7 +258,7 @@ describe('withViewModel', () => {
257258
test('container remounts VM with fixed id and some child with dynamic id', async ({
258259
task,
259260
}) => {
260-
const vmStore = new ViewModelStoreImpl();
261+
const vmStore = new ViewModelStoreMock();
261262

262263
class LayoutVM extends ViewModelMock {}
263264

src/view-model/view-model.impl.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,23 @@ export class ViewModelMock<
2222
super({
2323
...params,
2424
id: params?.id ?? '1',
25-
payload: params?.payload ?? ({} as any),
25+
payload: params?.payload as Payload,
2626
});
2727
}
2828

2929
didMount(): void {
30+
super.didMount();
3031
this.spies.didMount();
3132
}
3233

3334
mount(): void {
34-
super.mount();
3535
this.spies.mount();
36+
super.mount();
3637
}
3738

3839
unmount(): void {
39-
super.unmount();
4040
this.spies.unmount();
41+
super.unmount();
4142
}
4243

4344
payloadChanged(payload: any): void {
@@ -46,6 +47,7 @@ export class ViewModelMock<
4647

4748
didUnmount(): void {
4849
this.spies.didUnmount();
50+
super.didUnmount();
4951
}
5052
}
5153

src/view-model/view-model.store.impl.test.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,50 @@ import { AbstractViewModelParams } from './abstract-view-model.types';
66
import { ViewModel } from './view-model';
77
import { ViewModelMock } from './view-model.impl.test';
88
import { ViewModelStore } from './view-model.store';
9-
import { ViewModelGenerateIdConfig } from './view-model.store.types';
9+
import {
10+
ViewModelGenerateIdConfig,
11+
ViewModelLookup,
12+
} from './view-model.store.types';
1013
import { AnyViewModel } from './view-model.types';
1114

1215
import { ViewModelStoreImpl } from '.';
1316

1417
export class ViewModelStoreMock extends ViewModelStoreImpl {
1518
spies = {
1619
generateViewModelId: vi.fn(),
20+
get: vi.fn(),
1721
};
1822

23+
get _viewModels() {
24+
return this.viewModels;
25+
}
26+
get _linkedComponentVMClasses() {
27+
return this.linkedComponentVMClasses;
28+
}
29+
get _viewModelIdsByClasses() {
30+
return this.viewModelIdsByClasses;
31+
}
1932
get _instanceAttachedCount() {
2033
return this.instanceAttachedCount;
2134
}
35+
get _mountingViews() {
36+
return this.mountingViews;
37+
}
38+
get _unmountingViews() {
39+
return this.unmountingViews;
40+
}
2241

2342
generateViewModelId<VM extends ViewModel>(
2443
config: ViewModelGenerateIdConfig<VM>,
2544
): string {
2645
const result = super.generateViewModelId(config);
27-
this.spies.generateViewModelId(result, config);
46+
this.spies.generateViewModelId.mockReturnValue(result)(config);
47+
return result;
48+
}
49+
50+
get<T extends AnyViewModel>(vmLookup: Maybe<ViewModelLookup<T>>): T | null {
51+
const result = super.get<T>(vmLookup);
52+
this.spies.get.mockReturnValue(result)(vmLookup);
2853
return result;
2954
}
3055
}

0 commit comments

Comments
 (0)