pixijs / pixi-react

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

Check-canvas-instance-in-another-window #507

Open pie6k opened 4 months ago

pie6k commented 4 months ago
Description of change

Currently, when creating root and checking if target is HTMLCanvasElement we check it with target instanceof HTMLCanvasElement

It will break in quite rare-use case, when rendering content into another window (created with const newWindow = window.open() using React portal.

In this case - created canvas is instance of newWindow.HTMLCanvasElement, not global HTMLCanvasElement (or technically window.HTMLCanvasElement). As a result - the app will crash.

Use case for such a scenario is eg. creating multi-window Electron applications, when child windows are created using window.open(). I described this architecture in detail here: https://pietrasiak.com/creating-multi-window-electron-apps-using-react-portals

Fixes:

I added getIsCanvasElement function that is checking for this case and modified createRoot function to use it when checking if target is canvas.

Pre-Merge Checklist
trezy commented 4 months ago

Great find! I think there's some weirdness on the branch, tho. Can you grab the latest beta branch, cherry-pick your commit onto there, and update this PR? Once you do it should be a pretty quick merge.

trezy commented 3 months ago

@pie6k bumping this, as I'd love to get it merged soon. 😁