brianzinn / react-babylonjs

React for Babylon 3D engine
https://brianzinn.github.io/react-babylonjs/
824 stars 105 forks source link

'insertBefore is not a function' #92

Closed laeckerv closed 4 years ago

laeckerv commented 4 years ago

I'm running in an issue, where the reconciler is calling insertBefore and this function is is not implemented in the ReactBabylonJSHostConfig.ts.

Uncaught TypeError: insertBefore is not a function
    at insertOrAppendPlacementNode (react-reconciler.development.js:11698)
    at insertOrAppendPlacementNode (react-reconciler.development.js:11706)
    at commitPlacement (react-reconciler.development.js:11658)
    at commitMutationEffects (react-reconciler.development.js:14150)
    at HTMLUnknownElement.callCallback (react-reconciler.development.js:10632)
    at Object.invokeGuardedCallbackDev (react-reconciler.development.js:10681)
    at invokeGuardedCallback (react-reconciler.development.js:10733)
    at commitRootImpl (react-reconciler.development.js:13923)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority (react-reconciler.development.js:1752)
    at commitRoot (react-reconciler.development.js:13795)
    at finishSyncRender (react-reconciler.development.js:13193)
    at performSyncWorkOnRoot (react-reconciler.development.js:13179)
    at react-reconciler.development.js:1802
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority (react-reconciler.development.js:1752)
    at flushSyncCallbackQueueImpl (react-reconciler.development.js:1797)
    at flushSyncCallbackQueue (react-reconciler.development.js:1785)
    at scheduleUpdateOnFiber (react-reconciler.development.js:12585)
    at Object.updateContainer (react-reconciler.development.js:15852)
    at react-babylonjs.js:15
    at commitHookEffectListMount (react-dom.development.js:19731)
    at commitPassiveHookEffects (react-dom.development.js:19769)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at flushPassiveEffectsImpl (react-dom.development.js:22853)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushPassiveEffects (react-dom.development.js:22820)
    at performSyncWorkOnRoot (react-dom.development.js:21737)
    at react-dom.development.js:11089
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushSyncCallbackQueueImpl (react-dom.development.js:11084)
    at flushSyncCallbackQueue (react-dom.development.js:11072)
    at batchedUpdates$1 (react-dom.development.js:21862)
    at Object.notify (Subscription.js:19)
    at Subscription.notifyNestedSubs (Subscription.js:92)
    at Subscription.handleChangeWrapper (Subscription.js:97)
    at Object.dispatch (redux.js:222)
    at e (<anonymous>:1:40553)
    at createEpicMiddleware.js:54
    at SafeSubscriber.dispatch [as _next] (redux.js:638)
    at SafeSubscriber.__tryOrUnsub (Subscriber.ts:266)
    at SafeSubscriber.next (Subscriber.ts:208)
    at Subscriber._next (Subscriber.ts:140)
    at Subscriber.next (Subscriber.ts:100)
    at MergeMapSubscriber.notifyNext (mergeMap.ts:170)
    at SimpleInnerSubscriber._next (innerSubscribe.ts:31)

I can't provide a simple example to reproduce this behaviour, however i know for sure that the reconciler is executing the following line (before is not null): https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberCommitWork.new.js#L2494

I'm using the follwing versions: "@babylonjs/core": "^4.1.0", "@babylonjs/gui": "^4.1.0", "@babylonjs/inspector": "^4.1.0", "@babylonjs/loaders": "^4.1.0", "react-babylonjs": "^2.2.9", "react-reconciler": "^0.25.1",

Please let me know if i can provide any more information.

brianzinn commented 4 years ago

thanks for reporting @laeckerv - i'll try to make a repro. i think the DOM equivalent is adding a sibling in order before. I can probably simulate using sorted keys to make a repro and then I can fix. I am guessing you have something like:

....
{
    nodes.map((node: any) => (
      <box key={`box=${node.id}`} />
    ))
}

reactdom renderer just does parentInstance.insertBefore(child, beforeChild);

brianzinn commented 4 years ago

Should be working on latest version 2.2.11 - tested by inserting and re-ordering as posted above. Luckily was fairly easy to reproduce the error you were receiving :)

laeckerv commented 4 years ago

Hello @brianzinn , thank you so much for the quick response and fix.

Fist of all, you were right with the guessed scenario:

<transformNode name="tf_shower_world_offset">
{
    nodes.map((node: any) => (
      // this can actually evaluate to different components
      <box key={`box=${node.id}`} />
    ))
}
</transformNode>

But I don't think the insertBefore is implemented correctly 😢

The tf_door is one of the components that gets rerendered. As you can see it somehow is moved/inserted in the wrong position of the tree.

Before rerender

Screenshot 2020-10-28 at 17 22 52

After rerender:

Screenshot 2020-10-28 at 17 23 03
brianzinn commented 4 years ago

Thanks for sharing more details - hopefully can sort it out. How the Scene Explorer connects the JSX and how Babylon builds a scene hierarchy graph may not match what you are expecting. I just wrote a quick patch that worked on basic unparented meshes, but had already noted to myself that it would not work on GUI, because of how addControl works.

I would need to know more about tf-shower-world-offset - is it a TransformNode declared in JSX? It looks like when tf_door is re-added to the scene it does not recognize it as something it could parent to, so when it is re-added it then joins the main container. If you are not using keys in an array then the react reconciler will remove and dispose the meshes from your scene and re-create them. This renderer won't change your model parenting unless it is in JSX and then it has a way to look for a parent to attach to from the JSX hierarchy. Hope you can share some code as well to repro.

edit: I have re-read your code now (missed the <transformNode > part). I can try that out - may be due to me not calling a lifecycle method - kind of a way when objects are created to perform logic - for a mesh/node it should parent itself, if it is nested in another node/mesh.

brianzinn commented 4 years ago

OK - should be fixed. If you are not on React 17 then it will complain. I've decided to move support to React 17. You don't need the react-reconciler installed anymore either. I've moved it away from a peer dependency. If it's not fixed then I will make a new storybook to reproduce.

laeckerv commented 4 years ago

Thank you @brianzinn . Seems to work now 👍