Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modularize pixi-react to allow user defined versions of React and PIXI #398

Closed
baseten opened this issue Feb 3, 2023 · 2 comments · Fixed by #407
Closed

Modularize pixi-react to allow user defined versions of React and PIXI #398

baseten opened this issue Feb 3, 2023 · 2 comments · Fixed by #407
Assignees

Comments

@baseten
Copy link
Collaborator

baseten commented Feb 3, 2023

Spec

Rough map of current source to new modules

  • components -> pixi-react-components
  • helpers -> pixi-react
  • hoc + stage/provider-> pixi-react?
  • hooks -> pixi-react-components
  • reconciler -> pixi-react-fiber
  • render -> pixi-react-components
  • stage/index -> pixi-react-components
  • utils/element -> pixi-react-fiber
  • utils/element/PixiComponent -> pixi-react
  • utils/invariant -> pixi-react
  • utils/pixi -> pixi-react-components?
  • utils/props -> pixi-react-components?

Notes

  • We can probably rely on peer-deps to handle React imports, but anything that relies on a PIXI import most likely needs to exist within pixi-react-components to maximize compatibility (since the PIXI import API has changed considerably between versions).
  • At first glance it would seem that pixi-react-components should be thin and pixi-react should be fat, but it likely needs to be the other way around. pixi-react will pretty much expose only the PixiComponent custom component API and possibly some utils.
  • Virtually everything else will come from pixi-react-components bar the React Reconciler, which comes from pixi-react-fiber.
  • The React Reconciler also needs configuration, although most is static, createElement depends on TYPES_INJECTED which will be stored in pixi-react but filled up in pixi-react-components.

Proposed Usage

In a user's app they should setup pixi-react something like this:

import { configurePixiReact } from 'pixi-react';
import { configurePixiReactFiber } from 'pixi-react-fiber-v18';
import { configurePixiReactComponents } from 'pixi-react-components-v7';

export default configurePixiReact({
    configurePixiReactComponents,
    configurePixiReactFiber,
});

Implementation

The configuration function would look something like this:

function configurePixiReact({
    configurePixiReactComponents,
    configurePixiReactFiber,
})
{
    // PixiComponent updates TYPES_INJECTED (or new name since all will be), but this can basically be private and not
    // allow dupes?
    // invariant, PixiComponent, CHILDREN and TYPES_INJECTED come from pixi-react

    // getTextureFromProps also lives inside pixi-react-components even if not exported
    const {
        TYPES,
        createRoot,
        render,
        unmountComponentAtNode,
        Stage,
        applyDefaultProps,
        AppProvider,
        AppConsumer,
        AppContext,
        useTick,
        useApp,
        withPixiApp,
        withFilters,
        eventHandlers,
    } = configurePixiReactComponents({
        invariant,
        PixiComponent,
    });

    // All of the reconciler is basically static, except createElement which depends on TYPES_INJECTED, this configure step
    // basically just makes createElement and then wires everything up
    const PixiFiber = configurePixiReactFiber({
        CHILDREN,
        // updated to include pixi-react-components by configurePixiReactComponents, should we pass this around explicitly?
        TYPES_INJECTED,
        invariant,
        applyDefaultProps,
    });

    return {
        // From pixi-react
        PixiComponent,
        // From pixi-react-components
        TYPES,
        createRoot,
        render,
        unmountComponentAtNode,
        Stage,
        AppProvider,
        AppConsumer,
        AppContext,
        useTick,
        useApp,
        applyDefaultProps,
        // These could come from pixi-react, should they tie to PIXI or React version?
        withPixiApp,
        withFilters,
        eventHandlers,
        // From pixi-react-fiber
        PixiFiber,
    };
}
@baseten baseten self-assigned this Feb 3, 2023
@baseten
Copy link
Collaborator Author

baseten commented Feb 3, 2023

@Zyie once we release this I think perhaps we should write a note about what we officially support? We can obviously encourage users to write whatever pixi component packages they need for whichever versions of PIXI, but the combinations could get complicated quickly.

So far we've talked about supporting React 17+ and PIXI 6+, does that mean we support:

  • React 17 + PIXI 6
  • React 18 + PIXI 6
  • React 17 + PIXI 7
  • React 18 + PIXI 7

Combinations could obviously get complex quickly especially as more versions are released in the future.

I'm wondering whether on top of the user injectable API I've proposed above, we should also provide pre-configured pixi-react version(s) packaged as a single module. So for example @pixi/react v8.0.0 becomes a wrapper for the above API including React 18 and PIXI 7?

@Zyie
Copy link
Member

Zyie commented Feb 6, 2023

@Zyie once we release this I think perhaps we should write a note about what we officially support? We can obviously encourage users to write whatever pixi component packages they need for whichever versions of PIXI, but the combinations could get complicated quickly.

So far we've talked about supporting React 17+ and PIXI 6+, does that mean we support:

  • React 17 + PIXI 6
  • React 18 + PIXI 6
  • React 17 + PIXI 7
  • React 18 + PIXI 7

Yeah i think these should be what we support

So for example @pixi/react v8.0.0 becomes a wrapper for the above API including React 18 and PIXI 7?

Yeah this is a good approach. Makes it very simple for whoever wants the latest version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants