Skip to content

Commit 5188de0

Browse files
authored
fix(@typegpu/react): Cleanup on-demand root on unmount (#2554)
1 parent d4cad9b commit 5188de0

4 files changed

Lines changed: 173 additions & 3 deletions

File tree

.github/workflows/pkg-pr.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
run: pnpm nightly-build
3838

3939
- name: Publish (pkg.pr.new)
40-
run: pnpm exec pkg-pr-new publish './packages/typegpu' './packages/typegpu-noise' './packages/typegpu-cli' './packages/unplugin-typegpu' --json output.json --comment=off --pnpm --no-compact
40+
run: pnpm exec pkg-pr-new publish './packages/typegpu' './packages/typegpu-noise' './packages/typegpu-react' './packages/typegpu-cli' './packages/unplugin-typegpu' --json output.json --comment=off --pnpm --no-compact
4141
- name: Post or update comment
4242
uses: actions/github-script@v6
4343
with:

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"test:browser": "vitest run --browser.enabled --project browser",
3333
"test:browser:watch": "vitest --browser.enabled --project browser",
3434
"test:coverage": "vitest --coverage run",
35-
"nightly-build": "SKIP_TESTS=true pnpm --filter typegpu --filter @typegpu/noise --filter @typegpu/cli --filter unplugin-typegpu prepublishOnly --skip-publish-tag-check",
35+
"nightly-build": "SKIP_TESTS=true pnpm --filter typegpu --filter @typegpu/noise --filter @typegpu/react --filter @typegpu/cli --filter unplugin-typegpu prepublishOnly --skip-publish-tag-check",
3636
"changes": "tgpu-dev-cli changes"
3737
},
3838
"devDependencies": {

packages/typegpu-react/src/core/root-context.tsx

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import React, {
77
useState,
88
} from 'react';
99
import tgpu, { type TgpuRoot } from 'typegpu';
10+
import { useDeferredCleanup } from './helper-hooks.ts';
1011

1112
function attachPromiseStatus<T>(
1213
promise: PromiseLike<T> & {
@@ -59,18 +60,31 @@ type RootContextResult =
5960

6061
interface RootContext {
6162
initOrGetRoot(): RootContextResult;
63+
unmount(): void;
6264
}
6365

6466
class OwnRootContext implements RootContext {
6567
#result: RootContextResult | undefined;
68+
#destroyed: boolean = false;
6669

6770
initOrGetRoot(): RootContextResult {
71+
if (this.#destroyed) {
72+
console.warn(`[@typegpu/react]: Tried to init an already destroyed root.`);
73+
return { status: 'rejected', error: new Error('Already destroyed') };
74+
}
75+
6876
if (!this.#result) {
6977
this.#result = {
7078
status: 'pending',
7179
promise: tgpu.init().then(
7280
(root) => {
73-
this.#result = { status: 'resolved', value: root };
81+
if (this.#destroyed) {
82+
root.destroy();
83+
this.#result = undefined;
84+
} else {
85+
this.#result = { status: 'resolved', value: root };
86+
}
87+
7488
return root;
7589
},
7690
(error) => {
@@ -83,6 +97,15 @@ class OwnRootContext implements RootContext {
8397

8498
return this.#result;
8599
}
100+
101+
unmount(): void {
102+
this.#destroyed = true;
103+
104+
if (this.#result?.status === 'resolved') {
105+
this.#result.value.destroy();
106+
}
107+
this.#result = undefined;
108+
}
86109
}
87110

88111
class ExistingRootContext implements RootContext {
@@ -95,6 +118,10 @@ class ExistingRootContext implements RootContext {
95118
initOrGetRoot(): RootContextResult {
96119
return this.result;
97120
}
121+
122+
unmount(): void {
123+
// Nothing to do on unmount
124+
}
98125
}
99126

100127
/**
@@ -124,6 +151,10 @@ export const Root = ({ children, root }: RootProps) => {
124151
return undefined;
125152
}, [root]);
126153

154+
useDeferredCleanup(() => {
155+
ownCtx.unmount();
156+
});
157+
127158
return <rootContext.Provider value={existingRootCtx ?? ownCtx}>{children}</rootContext.Provider>;
128159
};
129160

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { act, render } from '@testing-library/react';
2+
import { afterEach, beforeEach, describe, expect, vi } from 'vitest';
3+
import tgpu from 'typegpu';
4+
import type { TgpuRoot } from 'typegpu';
5+
import { Root, useRootWithStatus } from '@typegpu/react';
6+
import { useEffect } from 'react';
7+
import { it } from './utils/extended-test.tsx';
8+
9+
describe('Root unmount cleanup', () => {
10+
beforeEach(() => {
11+
vi.useFakeTimers();
12+
});
13+
14+
afterEach(() => {
15+
vi.useRealTimers();
16+
});
17+
18+
it('should destroy own root on unmount (deferred cleanup)', async () => {
19+
let capturedRoot: TgpuRoot | undefined;
20+
21+
function TestConsumer() {
22+
const result = useRootWithStatus();
23+
24+
useEffect(() => {
25+
if (result.status === 'resolved') {
26+
capturedRoot = result.value;
27+
}
28+
});
29+
30+
return null;
31+
}
32+
33+
const { unmount } = render(
34+
<Root>
35+
<TestConsumer />
36+
</Root>,
37+
);
38+
39+
// Flush microtasks so tgpu.init() resolves and React re-renders
40+
await act(async () => {
41+
await Promise.resolve();
42+
});
43+
44+
expect(capturedRoot).toBeDefined();
45+
const destroySpy = vi.spyOn(capturedRoot!, 'destroy');
46+
47+
unmount();
48+
vi.runAllTimers();
49+
50+
expect(destroySpy).toHaveBeenCalledTimes(1);
51+
});
52+
53+
it('should not throw when existing root unmounts', async ({ RootWrapper }) => {
54+
const { unmount } = render(<div />, { wrapper: RootWrapper });
55+
expect(() => unmount()).not.toThrow();
56+
});
57+
58+
it('should destroy root when init promise resolves after unmount', async ({
59+
root: fixtureRoot,
60+
}) => {
61+
let resolveInit!: (root: TgpuRoot) => void;
62+
const initPromise = new Promise<TgpuRoot>((resolve) => {
63+
resolveInit = resolve;
64+
});
65+
66+
using _initSpy = vi.spyOn(tgpu, 'init').mockReturnValue(initPromise);
67+
const destroySpy = vi.spyOn(fixtureRoot, 'destroy');
68+
69+
function TestConsumer() {
70+
useRootWithStatus();
71+
return null;
72+
}
73+
74+
const { unmount } = render(
75+
<Root>
76+
<TestConsumer />
77+
</Root>,
78+
);
79+
80+
// Unmount and run deferred cleanup before init resolves
81+
unmount();
82+
vi.runAllTimers();
83+
84+
// Resolve init after context has been destroyed
85+
resolveInit(fixtureRoot);
86+
87+
// Flush microtasks so the .then() callback runs
88+
await act(async () => {
89+
await Promise.resolve();
90+
});
91+
92+
expect(destroySpy).toHaveBeenCalledTimes(1);
93+
});
94+
95+
describe('React StrictMode compatibility', () => {
96+
it('should survive the double mount/unmount cycle', async () => {
97+
let capturedRoot: TgpuRoot | undefined;
98+
99+
function TestConsumer() {
100+
const result = useRootWithStatus();
101+
102+
useEffect(() => {
103+
if (result.status === 'resolved') {
104+
capturedRoot = result.value;
105+
}
106+
});
107+
108+
return null;
109+
}
110+
111+
const { unmount } = render(
112+
<Root>
113+
<TestConsumer />
114+
</Root>,
115+
{ reactStrictMode: true },
116+
);
117+
118+
// Flush microtasks so tgpu.init() resolves and React re-renders
119+
await act(async () => {
120+
await Promise.resolve();
121+
});
122+
123+
expect(capturedRoot).toBeDefined();
124+
125+
// Run any pending deferred cleanup timeouts (from StrictMode's first mount)
126+
vi.runAllTimers();
127+
128+
// Root should still be alive (deferred cleanup was cancelled by remount)
129+
const destroySpy = vi.spyOn(capturedRoot!, 'destroy');
130+
expect(destroySpy).not.toHaveBeenCalled();
131+
132+
// Now actually unmount
133+
unmount();
134+
vi.runAllTimers();
135+
136+
expect(destroySpy).toHaveBeenCalledTimes(1);
137+
});
138+
});
139+
});

0 commit comments

Comments
 (0)