-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Introduce a hook to auto dispose view models #31178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| Copyright 2025 New Vector Ltd. | ||
| SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial | ||
| Please see LICENSE files in the repository root for full details. | ||
| */ | ||
|
|
||
| import { renderHook } from "jest-matrix-react"; | ||
|
|
||
| import { BaseViewModel } from "../BaseViewModel"; | ||
| import { useCreateAutoDisposedViewModel } from "../useCreateAutoDisposedViewModel"; | ||
|
|
||
| class TestViewModel extends BaseViewModel<{ count: number }, { initial: number }> { | ||
| public constructor(props: { initial: number }) { | ||
| super(props, { count: props.initial }); | ||
| } | ||
|
|
||
| public increment(): void { | ||
| const newCount = this.getSnapshot().count + 1; | ||
| this.snapshot.set({ count: newCount }); | ||
| } | ||
| } | ||
|
|
||
| describe("useAutoDisposedViewModel", () => { | ||
| it("should return view-model", () => { | ||
| const vmCreator = (): TestViewModel => new TestViewModel({ initial: 0 }); | ||
| const { result } = renderHook(() => useCreateAutoDisposedViewModel(vmCreator)); | ||
| const vm = result.current; | ||
| expect(vm).toBeInstanceOf(TestViewModel); | ||
| expect(vm.isDisposed).toStrictEqual(false); | ||
| }); | ||
|
|
||
| it("should dispose view-model on unmount", () => { | ||
| const vmCreator = (): TestViewModel => new TestViewModel({ initial: 0 }); | ||
| const { result, unmount } = renderHook(() => useCreateAutoDisposedViewModel(vmCreator)); | ||
| const vm = result.current; | ||
| vm.increment(); | ||
| unmount(); | ||
| expect(vm.isDisposed).toStrictEqual(true); | ||
| }); | ||
|
|
||
| it("should recreate view-model on react strict mode", async () => { | ||
| const vmCreator = (): TestViewModel => new TestViewModel({ initial: 0 }); | ||
| const output = renderHook(() => useCreateAutoDisposedViewModel(vmCreator), { reactStrictMode: true }); | ||
| const vm = output.result.current; | ||
| expect(vm.isDisposed).toStrictEqual(false); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| Copyright 2025 Element Creations Ltd. | ||
| SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial | ||
| Please see LICENSE files in the repository root for full details. | ||
| */ | ||
|
|
||
| import { useEffect, useState } from "react"; | ||
|
|
||
| import type { BaseViewModel } from "./BaseViewModel"; | ||
|
|
||
| type VmCreator<B extends BaseViewModel<unknown, unknown>> = () => B; | ||
|
|
||
| /** | ||
| * Instantiate a view-model that gets disposed when the calling react component unmounts. | ||
| * In other words, this hook ties the lifecycle of a view-model to the lifecycle of a | ||
| * react component. | ||
| * | ||
| * @param vmCreator A function that returns a view-model instance | ||
| * @returns view-model instance from vmCreator | ||
| * @example | ||
| * const vm = useCreateAutoDisposedViewModel(() => new FooViewModel({prop1, prop2, ...}); | ||
| */ | ||
| export function useCreateAutoDisposedViewModel<B extends BaseViewModel<unknown, unknown>>(vmCreator: VmCreator<B>): B { | ||
| /** | ||
| * The view-model instance may be replaced by a different instance in some scenarios. | ||
| * We want to be sure that whatever react component called this hook gets re-rendered | ||
| * when this happens, hence the state. | ||
| */ | ||
| const [viewModel, setViewModel] = useState<B>(vmCreator); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would a useRef be more appropriate maybe, since the view model isn't a thing that would need a re-render when it changes and so isn't really state? |
||
|
|
||
| /** | ||
| * Our intention here is to ensure that the dispose method of the view-model gets called | ||
| * when the component that uses this hook unmounts. | ||
| * We can do that by combining a useEffect cleanup with an empty dependency array. | ||
| */ | ||
| useEffect(() => { | ||
| let toDispose = viewModel; | ||
|
|
||
| /** | ||
| * Because we use react strict mode, react will run our effects twice in dev mode to make | ||
| * sure that they are pure. | ||
| * This presents a complication - the vm instance that we created in our state initializer | ||
| * will get disposed on the first cleanup. | ||
| * So we'll recreate the view-model if it's already disposed. | ||
| */ | ||
| if (viewModel.isDisposed) { | ||
| const newViewModel = vmCreator(); | ||
| // Change toDispose so that we don't end up disposing the already disposed vm. | ||
| toDispose = newViewModel; | ||
| setViewModel(newViewModel); | ||
| } | ||
| return () => { | ||
| // Dispose the view-model when this component unmounts | ||
| toDispose.dispose(); | ||
| }; | ||
|
|
||
| // eslint-disable-next-line react-compiler/react-compiler | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which dep are we deliberately not including and why? |
||
| }, []); | ||
|
|
||
| return viewModel; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this probably should be Element Creations? (Also could do with a new line below)