diff --git a/.changeset/bright-rings-read.md b/.changeset/bright-rings-read.md new file mode 100644 index 0000000000..72a1d8b7d0 --- /dev/null +++ b/.changeset/bright-rings-read.md @@ -0,0 +1,5 @@ +--- +"@lynx-js/react": patch +--- + +Fix crash caused by not removing event listeners during destroy. diff --git a/packages/react/runtime/__test__/lifecycle/destroy.test.jsx b/packages/react/runtime/__test__/lifecycle/destroy.test.jsx new file mode 100644 index 0000000000..517a1a0c24 --- /dev/null +++ b/packages/react/runtime/__test__/lifecycle/destroy.test.jsx @@ -0,0 +1,45 @@ +import { render } from 'preact'; +import { beforeAll, describe, expect, test, vi } from 'vitest'; + +describe('Destroy', () => { + const addEventListener = vi.fn(); + const removeEventListener = vi.fn(); + + beforeAll(() => { + lynx.getCoreContext = vi.fn(() => { + return { + addEventListener, + removeEventListener, + }; + }); + }); + + test('should remove event listener when throw in cleanup', async function() { + vi.resetModules(); + await import('../../src/lynx'); + + expect(addEventListener).toHaveBeenCalled(); + expect(removeEventListener).toHaveBeenCalledTimes(0); + + const { useEffect } = await import('../../src/index'); + const { __root } = await import('../../src/root'); + + const callback = vi.fn().mockImplementation(() => { + throw '???'; + }); + + function Comp() { + useEffect(() => callback, []); + return null; + } + + render(, __root); + await Promise.resolve().then(() => {}); + + expect(() => lynxCoreInject.tt.callDestroyLifetimeFun()).toThrow('???'); + + await Promise.resolve().then(() => {}); + expect(callback).toHaveBeenCalledTimes(1); + expect(removeEventListener).toHaveBeenCalledTimes(addEventListener.mock.calls.length); + }); +}); diff --git a/packages/react/runtime/__test__/utils/runtimeProxy.test.js b/packages/react/runtime/__test__/utils/runtimeProxy.test.js index 5492d5f3f3..ad34235507 100644 --- a/packages/react/runtime/__test__/utils/runtimeProxy.test.js +++ b/packages/react/runtime/__test__/utils/runtimeProxy.test.js @@ -17,6 +17,7 @@ describe('runtimeProxy', () => { globalEnvManager.switchToBackground(); lynx.getCoreContext().dispatchEvent({ type: 'event1', data: 'test' }); expect(event).toBeCalledWith({ type: 'event1', data: 'test' }); + lynx.getJSContext().removeEventListener('event1', event); }); it('should not trigger event when context mismatch', () => { @@ -24,6 +25,7 @@ describe('runtimeProxy', () => { lynx.getJSContext().addEventListener('event1', event); lynx.getJSContext().dispatchEvent({ type: 'event1', data: 'test' }); expect(event).not.toBeCalled(); + lynx.getJSContext().removeEventListener('event1', event); }); it('should throw when dispatch event to the same context', () => { diff --git a/packages/react/runtime/__test__/utils/setup.js b/packages/react/runtime/__test__/utils/setup.js index a3f2f08d42..4396c1e81e 100644 --- a/packages/react/runtime/__test__/utils/setup.js +++ b/packages/react/runtime/__test__/utils/setup.js @@ -17,8 +17,12 @@ function inject() { inject(); afterEach((context) => { - if (context.task.name.includes('preact/debug')) { + const skippedTasks = [ // Skip preact/debug tests since it would throw errors and abort the rendering process + 'preact/debug', + 'should remove event listener when throw in cleanup', + ]; + if (skippedTasks.some(task => context.task.name.includes(task))) { return; } diff --git a/packages/react/runtime/src/lynx/tt.ts b/packages/react/runtime/src/lynx/tt.ts index 4ffb1ae540..73acba5d99 100644 --- a/packages/react/runtime/src/lynx/tt.ts +++ b/packages/react/runtime/src/lynx/tt.ts @@ -30,9 +30,9 @@ function injectTt(): void { tt.publishEvent = delayedPublishEvent; tt.publicComponentEvent = delayedPublicComponentEvent; tt.callDestroyLifetimeFun = () => { + removeCtxNotFoundEventListener(); destroyWorklet(); destroyBackground(); - removeCtxNotFoundEventListener(); }; tt.updateGlobalProps = updateGlobalProps; tt.updateCardData = updateCardData;