pixijs / pixi-react

Write PIXI apps using React declarative style
https://pixijs.io/pixi-react/
MIT License
2.36k stars 179 forks source link

HostConfig.insertBefore not emitting `__REACT_PIXI_REQUEST_RENDER__` thus no rerender #408

Closed jsimomaa closed 6 months ago

jsimomaa commented 1 year ago

Current Behavior

When having a conditional child for a <Container> within a <Stage> the child does not reappear once removed.

Expected Behavior

The blue line should become visible again after clicking the button again.

Steps to Reproduce

HostConfig.insertBefore not emitting __REACT_PIXI_REQUEST_RENDER__ when a new child is inserted in to a <Container> within a <Stage raf={false}>:

  ...
  const toggledRect = toggled ? (
    <Rect x1={300} y1={300} x2={300} y2={150} />
  ) : null;

  return (
    <Stage raf={false} renderOnComponentChange={true}>
      <Container>
        {toggledRect}
  ...

See reproducible example at. The behavior can be seen when clicking the button : https://codesandbox.io/s/distracted-dust-4t4erp?file=/src/App.tsx

Environment

From the above sandbox:

  "dependencies": {
    "@pixi/core": "7.1.2",
    "@pixi/react": "7.0.1",
    "pixi.js": "7.1.2",
    "react": "18.0.0",
    "react-dom": "18.0.0",
    "react-scripts": "4.0.3"
  },

Possible Solution

All the other HostConfig functions that deal with mutation of children do emit __REACT_PIXI_REQUEST_RENDER__ and so should insertBefore as well in order to rerender the changed stage:

The relevant part in the code: https://github.com/pixijs/pixi-react/blob/82ae3e5c8a19794a1b518b6471b4f139dab80c4a/packages/react/src/reconciler/hostconfig.js#L102-L110

jsimomaa commented 1 year ago

Any comments if this actually is a bug and can be fixed with the solution below:

 function insertBefore(parent, child, beforeChild) 
 { 
     invariant(child !== beforeChild, 'pixi-react: PixiFiber cannot insert node before itself'); 

     const childExists = parent.children.indexOf(child) !== -1; 
     const index = parent.getChildIndex(beforeChild); 

     childExists ? parent.setChildIndex(child, index) : parent.addChildAt(child, index); 
     parent.__reactpixi?.root?.emit(`__REACT_PIXI_REQUEST_RENDER__`, { detail: 'insertBefore' });
}

Do you accept a PR for this?

rnike commented 1 year ago

We also encounter this issue, the solution @jsimomaa given does fix the bug.

avpeery commented 8 months ago

@jsimomaa I believe I ran into this same bug #469. I think pixi-react team should put in a fix for this rather than us implementing a workaround in the hostConfig file? Have you opened a PR yet? bumping up @baseten @bigtimebuddy thanks all!

jsimomaa commented 8 months ago

It seems like the development of this project halted after it become an official PixiJS package. It would be nice to get some kind of a comment from the PixiJS team about the status and the future of this project @baseten @bigtimebuddy @Zyie

The main pixijs project seems to be active but this React package is falling behind

Thanks!

Zyie commented 8 months ago

Hey @jsimomaa

We're a small team juggling priorities, and the v8 release for Pixi has been keeping us busy for a year now. We're planning to get the React package up to speed too with the v8 launch.

In the meantime this is an open source project, so If you're keen to chip in, feel free to submit PRs – we'd love the help!

jsimomaa commented 8 months ago

Hi @Zyie

Thank you for a quick response! Nice to hear that this React package is still maintained and will be brought up to speed with the v8!

I'll submit a PR for this issue ASAP like proposed in this comment: https://github.com/pixijs/pixi-react/issues/408#issuecomment-1471367967

jsimomaa commented 8 months ago

Hi @Zyie please see this PR: https://github.com/pixijs/pixi-react/pull/470

Thanks

OwenTrokeBillard commented 7 months ago

A workaround is to add onglobalpointermove={() => null} to your container. The callback will not be invoked as long as the container does not have interactive={true}.