pixijs / pixi-react

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

Feature Request: Expose root to injected components #419

Closed lunarraid closed 1 year ago

lunarraid commented 1 year ago

Description

Unlike native elements, injected elements are not exposed to the root, and are thus unable to use methods such as getTextureFromProps(), or emit change updates. Exposing the root could be as simple as this:

instance = injected.create(props, root);
lunarraid commented 1 year ago

It does look like root is exposed later in a dynamically added __reactpixi variable, but this is only done after the initial applyProps. Maybe it would be sufficient to set this before the initial applyProps?

On a further note, looks like getTextureFromProps itself is not exposed. Is this something that should be exposed for those creating custom components? It seems odd to have one setup for native components, and a completely different setup for externally injected ones.

michalochman commented 1 year ago

@lunarraid answering your latter note – there is a plan already to make unified solution for both native an injected components.

lunarraid commented 1 year ago

@michalochman What do you believe is the best path for the immediate term?

michalochman commented 1 year ago

@lunarraid, not sure if @baseten agrees with me, but looking at the current code I think we could implement an interim solution:

  1. Expose root to "injected" elements by changing the signature of create method to eg. instance = injected.create(props, { root });
  2. (optional) Change argument order of "built-in" elements from (root, props) to (props, { root }). This should not be a breaking change since they are internal to the library – what is exported are string names of built-in elements.

The reason I propose to use { root } instead of root as the second argument is that we could later extend it and not have to add more positional arguments.

By implementing (2) we would already have both types of elements using the same arguments when creating them, but this is not necessary now.

Another question is whether this solution is sufficient for your needs. Take note that there are other code paths that are different for built-in and injected elements

baseten commented 1 year ago

@lunarraid @michalochman hey both, sorry for the late response on this - I actually already implemented this in the new modular version, but as there is still quite a bit of work outstanding here I'll circle back and implement for v7 too.

I agree having { root } as second option is sensible 👍

lunarraid commented 1 year ago

@baseten What's the status on that modular version? I have a PR up for the above patch if it's still a ways out.