Skip to content
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

Destroy application on unmount #542

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
504aaa9
fix: make Application children optional
trezy Sep 2, 2024
58c58f7
Merge branch 'beta' into 521-bug-v8-memoized-component-renders-after-…
trezy Sep 3, 2024
b91fa7e
fix: handle unmounting `<Application>`
trezy Sep 3, 2024
110ba3d
test: add test for `<Application>`s `onInit`
trezy Sep 3, 2024
6ac48ac
ci: temp disable dev publish
trezy Sep 3, 2024
b370e5a
test: test `<Application>` unmounts
trezy Sep 3, 2024
e0b6e4b
chore: rename `unmountApplications` to `processUnmountedQueue` for cl…
trezy Sep 4, 2024
14635f1
chore: remove unnecessary optional operator
trezy Sep 4, 2024
62aae0b
chore: code style
trezy Sep 4, 2024
6429bb4
test: update `useTick` tests to use `expect.poll`
trezy Sep 4, 2024
f429cd6
chore: remove unused test util
trezy Sep 4, 2024
8e2ad70
refactor: check app state instead of eating the destroy error
trezy Sep 4, 2024
e629827
refactor: use a Set for unmount queue
trezy Sep 4, 2024
2592a34
refactor: rename `unmountApplication` to `unmountRoot` for clarity
trezy Sep 4, 2024
57434d8
test: improve unmounted app check
trezy Sep 4, 2024
78d6b96
test: verify applications are removed from the roots cache after unmount
trezy Dec 26, 2024
4d8f15b
fix: make sure applications always have a canvas in internal state
trezy Dec 26, 2024
53cddcd
build: update import attributes
trezy Dec 26, 2024
95197dd
build: remove unused dep
trezy Dec 26, 2024
4fc621b
ci: disable bin rebuild
trezy Dec 26, 2024
f3cc9e5
Merge branch 'beta' into 521-bug-v8-memoized-component-renders-after-…
trezy Dec 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions .github/workflows/handle-release-branch-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
run: npm run ${{ matrix.script.command }}

publish:
name: Publish
name: 'Publish: Release'
needs:
- verify
if: contains(fromJson('["refs/heads/alpha", "refs/heads/beta", "refs/heads/main"]'), github.ref)
Expand All @@ -49,14 +49,14 @@ jobs:
with:
fetch-depth: 0

- name: Setup Project
- name: Setup project
uses: ./.github/actions/setup

- name: Build Project
run: npm run build
shell: bash

- name: Semantic Release
- name: Semantic release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
id: release
Expand All @@ -71,3 +71,34 @@ jobs:
if: ${{ steps.release.outputs.new_tag_version != '' }}
run: npm publish ./dist/*.tgz --tag ${{ (github.head_ref || github.ref_name) == 'main' && 'latest' || github.head_ref || github.ref_name }}
shell: bash

# dev-publish:
# name: 'Publish: Dev'
# needs:
# - verify
# if: contains(fromJson('["refs/heads/alpha", "refs/heads/beta", "refs/heads/main"]'), github.ref) == 'false'
# runs-on: ubuntu-latest
# steps:
# - name: Checkout
# uses: actions/checkout@v4
# with:
# fetch-depth: 0

# - name: Setup project
# uses: ./.github/actions/setup

# - name: Build project
# run: npm run build
# shell: bash

# - name: Get current version
# run: echo "PACKAGE_VERSION=$(npm pkg get version | tr -d '"')" >> $GITHUB_ENV

# - name: Setup dev version
# run: echo "BRANCH_VERSION=$PACKAGE_VERSION-$BRANCH_NAME.${GITHUB_SHA::7}" >> $GITHUB_ENV

# - name: Bump version
# run: npm version $BRANCH_VERSION --no-git-tag-version --force

# - name: Publish a dev release
# run: npm publish --tag dev
38 changes: 30 additions & 8 deletions src/components/Application.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
type Application as PixiApplication,
extensions as PixiExtensions,
TextStyle,
} from 'pixi.js';
Expand All @@ -8,14 +9,16 @@ import {
type ForwardRefRenderFunction,
type MutableRefObject,
useCallback,
useEffect,
useRef,
} from 'react';
import { createRoot } from '../core/createRoot';
import { roots } from '../core/roots';
import { queueForUnmount } from '../helpers/queueForUnmount';
import { unmountApplications } from '../helpers/unmountApplications';
import { unqueueForUnmount } from '../helpers/unqueueForUnmount';
import { useIsomorphicLayoutEffect } from '../hooks/useIsomorphicLayoutEffect';
import { type ApplicationProps } from '../typedefs/ApplicationProps';
import { type Root } from '../typedefs/Root';

import type { Application as PixiApplication } from 'pixi.js';

const originalDefaultTextStyle = { ...TextStyle.defaultTextStyle };

Expand All @@ -39,7 +42,6 @@ export const ApplicationFunction: ForwardRefRenderFunction<PixiApplication, Appl
const applicationRef: MutableRefObject<PixiApplication | null> = useRef(null);
const canvasRef: MutableRefObject<HTMLCanvasElement | null> = useRef(null);
const extensionsRef: MutableRefObject<Set<any>> = useRef(new Set());
const rootRef: MutableRefObject<Root | null> = useRef(null);

const updateResizeTo = useCallback(() =>
{
Expand Down Expand Up @@ -71,6 +73,8 @@ export const ApplicationFunction: ForwardRefRenderFunction<PixiApplication, Appl

const handleInit = useCallback((application: PixiApplication) =>
{
unmountApplications();

if (forwardedRef && ('current' in forwardedRef))
{
forwardedRef.current = application;
Expand Down Expand Up @@ -136,12 +140,14 @@ export const ApplicationFunction: ForwardRefRenderFunction<PixiApplication, Appl

if (canvasElement)
{
if (!rootRef.current)
let root = roots.get(canvasElement);
trezy marked this conversation as resolved.
Show resolved Hide resolved

if (!root)
{
rootRef.current = createRoot(canvasElement, {}, handleInit);
root = createRoot(canvasElement, {}, handleInit);
}

rootRef.current.render(children, applicationProps);
root.render(children, applicationProps);
}
}, [
applicationProps,
Expand All @@ -167,9 +173,25 @@ export const ApplicationFunction: ForwardRefRenderFunction<PixiApplication, Appl
}
}, [defaultTextStyle]);

// eslint-disable-next-line consistent-return
useEffect(() =>
trezy marked this conversation as resolved.
Show resolved Hide resolved
{
const canvasElement = canvasRef.current;

if (canvasElement)
{
unqueueForUnmount(canvasElement);

return () =>
{
queueForUnmount(canvasElement);
};
}
}, []);

return createElement('canvas', {
ref: canvasRef,
className,
ref: canvasRef,
});
};

Expand Down
19 changes: 19 additions & 0 deletions src/helpers/queueForUnmount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { roots } from '../core/roots';
import { unmountApplication } from './unmountApplication';

export function queueForUnmount(canvas: HTMLCanvasElement)
{
const root = roots.get(canvas);

if (root)
{
if (root.applicationState.isInitialised)
{
unmountApplication(root);
}
else
{
root.internalState.queuedForUnmount = true;
}
}
}
27 changes: 27 additions & 0 deletions src/helpers/unmountApplication.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { reconciler } from '../core/reconciler';
import { roots } from '../core/roots';
import { type Root } from '../typedefs/Root';

export function unmountApplication(root: Root)
{
if (root.internalState.queuedForUnmount)
{
const fiber = root?.fiber;

trezy marked this conversation as resolved.
Show resolved Hide resolved
if (fiber)
{
reconciler.updateContainer(null, fiber, null, () =>
{
try
{
root.applicationState.app.destroy();
roots.delete(root.internalState.canvas!);
}
catch (error)
{
/* ... */
trezy marked this conversation as resolved.
Show resolved Hide resolved
}
});
}
}
}
10 changes: 10 additions & 0 deletions src/helpers/unmountApplications.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { roots } from '../core/roots';
import { unmountApplication } from './unmountApplication';

export function unmountApplications()
trezy marked this conversation as resolved.
Show resolved Hide resolved
{
for (const root of roots.values())
{
unmountApplication(root);
}
}
11 changes: 11 additions & 0 deletions src/helpers/unqueueForUnmount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { roots } from '../core/roots';

export function unqueueForUnmount(canvas: HTMLCanvasElement)
{
const root = roots.get(canvas);

if (root)
{
root.internalState.queuedForUnmount = false;
}
}
1 change: 1 addition & 0 deletions src/typedefs/InternalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ import type { HostConfig } from './HostConfig';
export interface InternalState
{
canvas?: HTMLCanvasElement;
queuedForUnmount?: boolean;
rootContainer: HostConfig['containerInstance'];
}
33 changes: 33 additions & 0 deletions test/e2e/components/Application.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {
describe,
expect,
it,
vi,
} from 'vitest';
import { render } from '@testing-library/react'

import { Application } from '../../../src/components/Application'
import { wait } from '../../utils/wait';

describe('Application', () =>
{
describe('onInit', () => {
trezy marked this conversation as resolved.
Show resolved Hide resolved
it('runs the callback', async () => {
const onInitSpy = vi.fn()

const TestComponent = function() {
return (
<Application onInit={onInitSpy} />
)
}

render((
<TestComponent />
))

await wait(1000)

expect(onInitSpy.mock.calls.length).to.equal(1)
});
})
});