Modular Packages

[baseten]

Resolves: #398

This PR splits out the core parts of @pixi/react to enable users to use the package with mix 'n' match versions of react-reconciler and pixi.js

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 504fe15:

Sandbox	Source
sandbox	Configuration

[Zyie]

Does this new set up work with react 17 and pixi 6?

I don't see either of these packages here

[michalochman]

@Zyie we have initially discussed with @baseten to implement react 18 and pixi 6 in first pass, because for previous versions users can work with already published package.

The solution in this PR looks good after first pass. I have not tried to run it yet.

However, now my thoughts are similar to @Zyie. Since there's only react 18 and pixi 7 implemented, I wonder how much code duplication there would be when implementing other versions with how it is split up currently.

I was initially questioning whether it is also better to check this now, by adding implementation for react 17 and pixi 6 or wait for the next version of either, but since people are already using this library with react 17 and pixi 6, merging this PR would be a breaking change for them.

[baseten]

@michalochman @Zyie Thanks both for the comments. So at this point, there are no backwards compatible breaking changes in the React Reconciler between React 17/18 or the PIXI API between 6 and 7, so the existing implementation should work for both of them as is. We could still implement separate versions for those libraries for consistency and as proof of compatibility. For React we'd likely just remove the createRoot API and tweak a couple of Reconciler methods. For PIXI we could remove the check for interaction manager in the Stage for v7.

@michalochman I agree that were someone to want to implement PIXI components for another version then there would currently be a lot of code duplication - I'm torn about how to handle this, my intention with react-components-pixi-v7 was that everything that depended on a PIXI import would be self contained in there, which means in theory you could implement this module for any version of PIXI going back a long way. Maybe it doesn't matter since the consumer won't import all that code, but potentially we could split this module into smaller pieces, the actual components themselves and then everything else - but of course this is more code complexity too. The issue about trying to share anything between versions is if the names of pixi modules / import paths changes, which is possible.

[Zyie]

There are no backwards compatible breaking changes in the React Reconciler between React 17/18 or the PIXI API between 6 and 7, so the existing implementation should work for both of them as is.

I do believe there was a breaking change in that react-reconciler bumped its peer-dependencies to react 18. I think 0.26.0 was the last version to use react 17

https://github.com/facebook/react/blob/main/packages/react-reconciler/package.json#L29

So this change would affect react 17 users with strict peer dependencies

[michalochman]

@baseten I was mostly referring to code duplication in the following areas.

React side:

• Files in packages/react-components-pixi-7/src/hooks/ directory and its siblings
• packages/react-fiber-react-18/src/diffProperties.js file could possibly be common to different React reconciler versions

PIXI side:

• Files in the packages/react-components-pixi-7/src/utils/ directory

Additionally, we have PIXI code in the React package. For example doRemoveChild in host config references children, texture and baseTexture. This is internal PIXI API that could change in the future. Heck, even method names like removeChild or destroy could change. The same issue can be observed in React hooks. Perhaps there is a way to provide them in some bridge code during setup?

There's probably a limit to which we should think about making the library modular, but I'm not sure yet where to draw the line.

[baseten]

There are no backwards compatible breaking changes in the React Reconciler between React 17/18 or the PIXI API between 6 and 7, so the existing implementation should work for both of them as is.

I do believe there was a breaking change in that react-reconciler bumped its peer-dependencies to react 18. I think 0.26.0 was the last version to use react 17

https://github.com/facebook/react/blob/main/packages/react-reconciler/package.json#L29

So this change would affect react 17 users with strict peer dependencies

@Zyie Good point, perhaps this is best addressed in a follow up PR? It will also be helpful in better determining modular package boundaries

[baseten]

@baseten I was mostly referring to code duplication in the following areas.

React side:

• Files in packages/react-components-pixi-7/src/hooks/ directory and its siblings
• packages/react-fiber-react-18/src/diffProperties.js file could possibly be common to different React reconciler versions

PIXI side:

• Files in the packages/react-components-pixi-7/src/utils/ directory

Additionally, we have PIXI code in the React package. For example doRemoveChild in host config references children, texture and baseTexture. This is internal PIXI API that could change in the future. Heck, even method names like removeChild or destroy could change. The same issue can be observed in React hooks. Perhaps there is a way to provide them in some bridge code during setup?

There's probably a limit to which we should think about making the library modular, but I'm not sure yet where to draw the line.

@michalochman I totally agree and I too think there can be some increased modularity on the PIXI component side, I will likely come back to this on a second pass.

In terms of PIXI code in React, for now I think having a minimal shared contract between the React Reconciler and PIXI is the simplest way to go, but yes we could write an injectable adapter in the future if necessary.

I have tried to keep this as small as possible, hence the following interfaces: 960985b#diff-a775e5ddfa88527e4061b30477f579be050f33464e6df56ceb79ba1a7b9af9fdR7-R23

@Zyie @GoodBoyDigital do you envisage these minimal interfaces changing in the near future? Or have they changed from previous versions of PIXI as far as you know?

[michalochman]

@baseten the minimal contract looks good. I am also perfectly fine with increasing modularity in another pass.