Bug in Hooks: ReactPixiFiber did not render into container provided

[Othreumaru]

Description

I believe around version 0.10.0 with PR
#127

A subtle bug was introduced. It can occur when react renders a component then that render triggers a change for example layout resize which triggers rerender in a sync way. Which causes fiber to unmount previous component which throws exception

ReactPixiFiber did not render into container provided

This happens because in hooks.js inside usePixiAppCreator inside useLayoutEffect you return a function that expects that component was already mounted. However not is perfectly normal in react for that method to be called before component is mounted.

Steps to reproduce

1. Render something that changes state and causes another render in same stack

Additional info

• react-pixi-fiber version: 0.10.0
• React version: ^16.10.1
• ReactDOM version: ^16.10.1
• PixiJS version: ^5.1.5

[michalochman]

@Othreumaru can you reproduce this also in 0.12.0? If you could provide some code sandbox with this happening that would be great. You can use this template as a starter.

[Othreumaru]

I have narrowed down the root cause of my problem to be options object changing on Stage causing a circular unmount loop since that was triggering update to my context. My case might be rare so feel free to close this.

[michalochman]

I can see if it is possible to eliminate such unmount loop if you could share a minimal options object that could be used to reproduce that scenario.

[nstolpe]

I'm pretty sure I'm also running into this issue. I'll try to recreate it and fix it.

[michalochman]

@nstolpe I have changed how it works significantly in #47 so not sure if it's worth fixing on master unless this is a common issue.

[nstolpe]

Actually, I think my issue has something to do with my app code. This example of changing backgroundColor in Stage options works fine. It's just useState though, no Context. I have tested locally with a Context and that's fine too, I'll try to est up an example for that as well. @michalochman if @Othreumaru can get us an example that reproduces the issue I'm happy to help look into resolving it.

[nstolpe]

@michalochman oh wow, you've made some major changes. I didn't see your reply until after posting my last one. If there are any outstanding issues with this after your refactor, or if you just need some testing done, I can help out.

[vojto]

Getting the same error. Not sure what's going on, because it's part of pretty complex app.

Any update on #47 getting merged in?

[nstolpe]

@vojto I've encountered another issue that seems related to this one. To work around it until 47 is merged, you can try the workaround I've been using, which is to use the PXI Stage class instead. Import createStageClass and call it to get your Stage component.

[vojto]

@nstolpe Thanks for that. I just got #47 running locally, so I'm gonna play with that for a while, and if it fails, I'll try your approach.

FYI for anyone who wants to run this from another branch. I didn't have much luck adding it as Yarn dependency from Git, so what I ended up doing was just opening my node_modules/ and replacing contents of cjs with a copy of built files from another branch.

[michalochman]

@vojto I am pretty happy with #47, however I'm a little afraid that the way it works now may give a lot of users heart attacks due to the amount of warnings – I have decided to make these warnings opt-in instead, but extra work will be needed. I'm currently exploring React.StrictMode and if that ends well, I'll be pretty much ready to release 1.0.0

[michalochman]

I have just released [email protected] (with #47 merged) if anyone wants to give it a try.

[michalochman]

@vojto @nstolpe were you able to fix your issues with that version? Can we close this issue?

I hope there were no braking changes introduced other than _app variable available on class-based Stage ref being converted to React.createRef.

[maerzhase]

bug seems to be resolved with [email protected]. I can't reproduce it anymore.

[vojto]

Works very well for me.

[nstolpe]

@michalochman sorry, just got some time to work on my project. Both issues I mentioned here seem to have been cleared up now.

[nstolpe]

@michalochman I've been using 1.0.0-beta.1 for a while now, but see that master has been updated more recently. Is the beta still the best branch to stick with?

[michalochman]

@nstolpe I will update the beta branch today if you're an active user.

There are several reasons it has staled:

• the production bundle size is about 10 kB larger than current version (before gzip) and I'm evaluating if it should be that way
• react-dom itself has shifted away from aggressive props validation which was the driver behind major update (and the base of it, so I'll have to review react-dom code again)
• there is an open question how to deal with props introduced by PixiJS plugins

[michalochman]

Turns out it will take me more time to rebase beta to master (looks like I missed some functionality there, like providing app prop to Stage), but I've started it already so expect it soon.

[michalochman]

@nstolpe I have just published 1.0.0-beta.2 with all features that were missing from the current branch.

[nstolpe]

@michalochman thanks for the update and for doing that so fast. I'll update to the new beta tonight.

[linonetwo]

I can confirm "react-pixi-fiber": "^1.0.0-beta.4", works very well!

Now hot module replacement won't break my game anymore.

[michalochman]

[email protected] released.