BitmapText errors on unmount

baseten commented 1 year ago

Description

The react reconciler cleanup code auto cleans up children, but in the case of BitmapText this causes an issue where BitmapText itself expects to cleanup its underlying Mesh instance:

https://github.com/pixijs/pixi-react/blob/d5f5773944d22f66d735143d76791e3f06690d9c/packages/react-pixi/src/reconciler/hostconfig.js#L31-L53

Screenshot 2022-12-30 at 12 05 46

According to the react-reconciler docs there is no need to traverse the subtree to destroy it.

removeChild(parentInstance, child) This method should mutate the parentInstance to remove the child from the list of its children.

React will only call it for the top-level node that is being removed. It is expected that garbage collection would take care of the > whole subtree. You are not expected to traverse the child tree in it.

There could potentially be PIXI cleanup that is necessary, but we could delegate that to PIXI.Container.destroy

Steps to reproduce

Render BitmapText instance
unmount it

Additional info

@pixi/react-pixi version: v7.0.0-alpha
React version: v18.2.0
ReactDOM version: v18.2.0
PIXI version: v6.5.8

baseten commented 1 year ago

Full console error in text:

react-reconciler.development.js?7719:14465 Uncaught TypeError: Cannot use 'in' operator to search for 'texture' in null
    at Mesh.get (mesh.mjs?7a80:244:1)
    at eval (text-bitmap.mjs?0ae8:1487:1)
    at Array.filter (<anonymous>)
    at BitmapText.destroy (text-bitmap.mjs?0ae8:1487:1)
    at _removeChild (hostconfig.js?99c2:39:1)
    at removeChildFromContainer (hostconfig.js?99c2:253:1)
    at commitDeletionEffectsOnFiber (react-reconciler.development.js?7719:15670:1)
    at recursivelyTraverseDeletionEffects (react-reconciler.development.js?7719:15631:1)
    at commitDeletionEffectsOnFiber (react-reconciler.development.js?7719:15705:1)
    at recursivelyTraverseDeletionEffects (react-reconciler.development.js?7719:15631:1)
get @ mesh.mjs?7a80:244
eval @ text-bitmap.mjs?0ae8:1487
BitmapText.destroy @ text-bitmap.mjs?0ae8:1487
_removeChild @ hostconfig.js?99c2:39
removeChildFromContainer @ hostconfig.js?99c2:253
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15670
recursivelyTraverseDeletionEffects @ react-reconciler.development.js?7719:15631
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15705
recursivelyTraverseDeletionEffects @ react-reconciler.development.js?7719:15631
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15807
commitDeletionEffects @ react-reconciler.development.js?7719:15585
recursivelyTraverseMutationEffects @ react-reconciler.development.js?7719:15905
commitMutationEffectsOnFiber @ react-reconciler.development.js?7719:16070
commitMutationEffects @ react-reconciler.development.js?7719:15892
commitRootImpl @ react-reconciler.development.js?7719:18949
commitRoot @ react-reconciler.development.js?7719:18819
performSyncWorkOnRoot @ react-reconciler.development.js?7719:18222
flushSyncCallbacks @ react-reconciler.development.js?7719:2930
eval @ react-reconciler.development.js?7719:17756
setTimeout (async)
eval @ useWindowSize.ts?9fc6:46
commitHookEffectListMount @ react-dom.development.js?3c4a:23150
commitPassiveMountOnFiber @ react-dom.development.js?3c4a:24931
commitPassiveMountEffects_complete @ react-dom.development.js?3c4a:24891
commitPassiveMountEffects_begin @ react-dom.development.js?3c4a:24878
commitPassiveMountEffects @ react-dom.development.js?3c4a:24866
flushPassiveEffectsImpl @ react-dom.development.js?3c4a:27039
flushPassiveEffects @ react-dom.development.js?3c4a:26984
eval @ react-dom.development.js?3c4a:26769
workLoop @ scheduler.development.js?ba31:266
flushWork @ scheduler.development.js?ba31:239
performWorkUntilDeadline @ scheduler.development.js?ba31:533
react-reconciler.development.js?7719:9860 The above error occurred in the <Context.Provider> component:

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.
logCapturedError @ react-reconciler.development.js?7719:9860
update.callback @ react-reconciler.development.js?7719:9886
callCallback @ react-reconciler.development.js?7719:5079
commitUpdateQueue @ react-reconciler.development.js?7719:5105
commitLayoutEffectOnFiber @ react-reconciler.development.js?7719:15002
commitLayoutMountEffects_complete @ react-reconciler.development.js?7719:16340
commitLayoutEffects_begin @ react-reconciler.development.js?7719:16340
commitLayoutEffects @ react-reconciler.development.js?7719:16278
commitRootImpl @ react-reconciler.development.js?7719:18962
commitRoot @ react-reconciler.development.js?7719:18819
performSyncWorkOnRoot @ react-reconciler.development.js?7719:18222
flushSyncCallbacks @ react-reconciler.development.js?7719:2930
eval @ react-reconciler.development.js?7719:17756
setTimeout (async)
eval @ useWindowSize.ts?9fc6:46
commitHookEffectListMount @ react-dom.development.js?3c4a:23150
commitPassiveMountOnFiber @ react-dom.development.js?3c4a:24931
commitPassiveMountEffects_complete @ react-dom.development.js?3c4a:24891
commitPassiveMountEffects_begin @ react-dom.development.js?3c4a:24878
commitPassiveMountEffects @ react-dom.development.js?3c4a:24866
flushPassiveEffectsImpl @ react-dom.development.js?3c4a:27039
flushPassiveEffects @ react-dom.development.js?3c4a:26984
eval @ react-dom.development.js?3c4a:26769
workLoop @ scheduler.development.js?ba31:266
flushWork @ scheduler.development.js?ba31:239
performWorkUntilDeadline @ scheduler.development.js?ba31:533
react-reconciler.development.js?7719:2949 Uncaught TypeError: Cannot use 'in' operator to search for 'texture' in null
    at Mesh.get (mesh.mjs?7a80:244:1)
    at eval (text-bitmap.mjs?0ae8:1487:1)
    at Array.filter (<anonymous>)
    at BitmapText.destroy (text-bitmap.mjs?0ae8:1487:1)
    at _removeChild (hostconfig.js?99c2:39:1)
    at removeChildFromContainer (hostconfig.js?99c2:253:1)
    at commitDeletionEffectsOnFiber (react-reconciler.development.js?7719:15670:1)
    at recursivelyTraverseDeletionEffects (react-reconciler.development.js?7719:15631:1)
    at commitDeletionEffectsOnFiber (react-reconciler.development.js?7719:15705:1)
    at recursivelyTraverseDeletionEffects (react-reconciler.development.js?7719:15631:1)
get @ mesh.mjs?7a80:244
eval @ text-bitmap.mjs?0ae8:1487
BitmapText.destroy @ text-bitmap.mjs?0ae8:1487
_removeChild @ hostconfig.js?99c2:39
removeChildFromContainer @ hostconfig.js?99c2:253
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15670
recursivelyTraverseDeletionEffects @ react-reconciler.development.js?7719:15631
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15705
recursivelyTraverseDeletionEffects @ react-reconciler.development.js?7719:15631
commitDeletionEffectsOnFiber @ react-reconciler.development.js?7719:15807
commitDeletionEffects @ react-reconciler.development.js?7719:15585
recursivelyTraverseMutationEffects @ react-reconciler.development.js?7719:15905
commitMutationEffectsOnFiber @ react-reconciler.development.js?7719:16070
commitMutationEffects @ react-reconciler.development.js?7719:15892
commitRootImpl @ react-reconciler.development.js?7719:18949
commitRoot @ react-reconciler.development.js?7719:18819
performSyncWorkOnRoot @ react-reconciler.development.js?7719:18222
flushSyncCallbacks @ react-reconciler.development.js?7719:2930
eval @ react-reconciler.development.js?7719:17756
setTimeout (async)
eval @ useWindowSize.ts?9fc6:46
commitHookEffectListMount @ react-dom.development.js?3c4a:23150
commitPassiveMountOnFiber @ react-dom.development.js?3c4a:24931
commitPassiveMountEffects_complete @ react-dom.development.js?3c4a:24891
commitPassiveMountEffects_begin @ react-dom.development.js?3c4a:24878
commitPassiveMountEffects @ react-dom.development.js?3c4a:24866
flushPassiveEffectsImpl @ react-dom.development.js?3c4a:27039
flushPassiveEffects @ react-dom.development.js?3c4a:26984
eval @ react-dom.development.js?3c4a:26769
workLoop @ scheduler.development.js?ba31:266
flushWork @ scheduler.development.js?ba31:239
performWorkUntilDeadline @ scheduler.development.js?ba31:533
core.mjs?6033:793 BaseTexture added to the cache with an id [https://s3-us-west-2.amazonaws.com/s.cdpn.io/693612/desyrel.png] that already had an entry
BaseTexture.addToCache @ core.mjs?6033:793
Texture.fromLoader @ core.mjs?6033:2551
TextureLoader.use @ loaders.mjs?2f12:1793
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._onLoad @ loaders.mjs?2f12:1643
Signal.dispatch @ loaders.mjs?2f12:120
LoaderResource._finish @ loaders.mjs?2f12:627
LoaderResource.complete @ loaders.mjs?2f12:490
load (async)
LoaderResource._loadElement @ loaders.mjs?2f12:652
LoaderResource.load @ loaders.mjs?2f12:554
eval @ loaders.mjs?2f12:1616
next @ loaders.mjs?2f12:1276
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._loadResource @ loaders.mjs?2f12:1604
Loader._boundLoadResource @ loaders.mjs?2f12:1402
AsyncQueue.process @ loaders.mjs?2f12:1184
setTimeout (async)
AsyncQueue._insert @ loaders.mjs?2f12:1172
AsyncQueue.push @ loaders.mjs?2f12:1225
Loader._add @ loaders.mjs?2f12:1467
add @ loaders.mjs?2f12:1730
BitmapFontLoader.use @ text-bitmap.mjs?0ae8:1580
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
parsing @ loaders.mjs?2f12:1919
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
TextureLoader.use @ loaders.mjs?2f12:1802
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._onLoad @ loaders.mjs?2f12:1643
Signal.dispatch @ loaders.mjs?2f12:120
LoaderResource._finish @ loaders.mjs?2f12:627
LoaderResource.complete @ loaders.mjs?2f12:490
LoaderResource._xhrOnLoad @ loaders.mjs?2f12:872
load (async)
LoaderResource._loadXhr @ loaders.mjs?2f12:732
LoaderResource.load @ loaders.mjs?2f12:574
eval @ loaders.mjs?2f12:1616
next @ loaders.mjs?2f12:1276
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._loadResource @ loaders.mjs?2f12:1604
Loader._boundLoadResource @ loaders.mjs?2f12:1402
AsyncQueue.process @ loaders.mjs?2f12:1184
AsyncQueue.resume @ loaders.mjs?2f12:1260
Loader.load @ loaders.mjs?2f12:1542
eval @ PIXILoader.js?de4a:1
commitHookEffectListMount @ react-reconciler.development.js?7719:14770
commitPassiveMountOnFiber @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects_complete @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects_begin @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects @ react-reconciler.development.js?7719:16533
flushPassiveEffectsImpl @ react-reconciler.development.js?7719:19164
flushPassiveEffects @ react-reconciler.development.js?7719:19118
commitRootImpl @ react-reconciler.development.js?7719:19077
commitRoot @ react-reconciler.development.js?7719:18819
performSyncWorkOnRoot @ react-reconciler.development.js?7719:18222
flushSyncCallbacks @ react-reconciler.development.js?7719:2930
eval @ react-reconciler.development.js?7719:17756
setTimeout (async)
eval @ useWindowSize.ts?9fc6:46
commitHookEffectListMount @ react-dom.development.js?3c4a:23150
commitPassiveMountOnFiber @ react-dom.development.js?3c4a:24931
commitPassiveMountEffects_complete @ react-dom.development.js?3c4a:24891
commitPassiveMountEffects_begin @ react-dom.development.js?3c4a:24878
commitPassiveMountEffects @ react-dom.development.js?3c4a:24866
flushPassiveEffectsImpl @ react-dom.development.js?3c4a:27039
flushPassiveEffects @ react-dom.development.js?3c4a:26984
eval @ react-dom.development.js?3c4a:26769
workLoop @ scheduler.development.js?ba31:266
flushWork @ scheduler.development.js?ba31:239
performWorkUntilDeadline @ scheduler.development.js?ba31:533
core.mjs?6033:2579 Texture added to the cache with an id [https://s3-us-west-2.amazonaws.com/s.cdpn.io/693612/desyrel.png] that already had an entry
Texture.addToCache @ core.mjs?6033:2579
Texture.fromLoader @ core.mjs?6033:2552
TextureLoader.use @ loaders.mjs?2f12:1793
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._onLoad @ loaders.mjs?2f12:1643
Signal.dispatch @ loaders.mjs?2f12:120
LoaderResource._finish @ loaders.mjs?2f12:627
LoaderResource.complete @ loaders.mjs?2f12:490
load (async)
LoaderResource._loadElement @ loaders.mjs?2f12:652
LoaderResource.load @ loaders.mjs?2f12:554
eval @ loaders.mjs?2f12:1616
next @ loaders.mjs?2f12:1276
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._loadResource @ loaders.mjs?2f12:1604
Loader._boundLoadResource @ loaders.mjs?2f12:1402
AsyncQueue.process @ loaders.mjs?2f12:1184
setTimeout (async)
AsyncQueue._insert @ loaders.mjs?2f12:1172
AsyncQueue.push @ loaders.mjs?2f12:1225
Loader._add @ loaders.mjs?2f12:1467
add @ loaders.mjs?2f12:1730
BitmapFontLoader.use @ text-bitmap.mjs?0ae8:1580
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
parsing @ loaders.mjs?2f12:1919
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
TextureLoader.use @ loaders.mjs?2f12:1802
eval @ loaders.mjs?2f12:1644
eval @ loaders.mjs?2f12:1282
setTimeout (async)
next @ loaders.mjs?2f12:1281
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._onLoad @ loaders.mjs?2f12:1643
Signal.dispatch @ loaders.mjs?2f12:120
LoaderResource._finish @ loaders.mjs?2f12:627
LoaderResource.complete @ loaders.mjs?2f12:490
LoaderResource._xhrOnLoad @ loaders.mjs?2f12:872
load (async)
LoaderResource._loadXhr @ loaders.mjs?2f12:732
LoaderResource.load @ loaders.mjs?2f12:574
eval @ loaders.mjs?2f12:1616
next @ loaders.mjs?2f12:1276
AsyncQueue.eachSeries @ loaders.mjs?2f12:1289
Loader._loadResource @ loaders.mjs?2f12:1604
Loader._boundLoadResource @ loaders.mjs?2f12:1402
AsyncQueue.process @ loaders.mjs?2f12:1184
AsyncQueue.resume @ loaders.mjs?2f12:1260
Loader.load @ loaders.mjs?2f12:1542
eval @ PIXILoader.js?de4a:1
commitHookEffectListMount @ react-reconciler.development.js?7719:14770
commitPassiveMountOnFiber @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects_complete @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects_begin @ react-reconciler.development.js?7719:16533
commitPassiveMountEffects @ react-reconciler.development.js?7719:16533
flushPassiveEffectsImpl @ react-reconciler.development.js?7719:19164
flushPassiveEffects @ react-reconciler.development.js?7719:19118
commitRootImpl @ react-reconciler.development.js?7719:19077
commitRoot @ react-reconciler.development.js?7719:18819
performSyncWorkOnRoot @ react-reconciler.development.js?7719:18222
flushSyncCallbacks @ react-reconciler.development.js?7719:2930
eval @ react-reconciler.development.js?7719:17756
setTimeout (async)
eval @ useWindowSize.ts?9fc6:46
commitHookEffectListMount @ react-dom.development.js?3c4a:23150
commitPassiveMountOnFiber @ react-dom.development.js?3c4a:24931
commitPassiveMountEffects_complete @ react-dom.development.js?3c4a:24891
commitPassiveMountEffects_begin @ react-dom.development.js?3c4a:24878
commitPassiveMountEffects @ react-dom.development.js?3c4a:24866
flushPassiveEffectsImpl @ react-dom.development.js?3c4a:27039
flushPassiveEffects @ react-dom.development.js?3c4a:26984
eval @ react-dom.development.js?3c4a:26769
workLoop @ scheduler.development.js?ba31:266
flushWork @ scheduler.development.js?ba31:239
performWorkUntilDeadline @ scheduler.development.js?ba31:533
mesh.mjs?7a80:247 Uncaught TypeError: Cannot set properties of null (setting 'texture')
    at Mesh.set (mesh.mjs?7a80:247:1)
    at BitmapText.updateText (text-bitmap.mjs?0ae8:1085:1)
    at BitmapText.validate (text-bitmap.mjs?0ae8:1249:1)
    at BitmapText.updateTransform (text-bitmap.mjs?0ae8:1204:1)
    at Container.updateTransform (display.mjs?15bf:1491:1)
    at Renderer.render (core.mjs?6033:9369:1)
    at Application.render (app.mjs?480c:209:1)
    at TickerListener.emit (ticker.mjs?f0fc:95:1)
    at Ticker.update (ticker.mjs?f0fc:458:1)
    at Ticker._tick (ticker.mjs?f0fc:226:1)

baseten commented 1 year ago

@inlet do you have any context you can share here? Was there a specific issue you were solving by recursing children?

inlet commented 1 year ago

@baseten sorry I didn’t respond earlier.. so many Github notifications, I must have missed this one for sure. I guess you already solved this one as the PR is closed right? It’s been a while since I wrote this code but yes there was a reason for it

pixijs / pixi-react