Expose injectIntoDevTools() to renderers

[gaearon]

An alternative to #11445.
This lets third party renderers inject themselves into the devtools.

Previously, renderers had to reach into reconciler directly to do this.
However, this wouldn't work for third party renderers.

Instead, I put injectIntoDevTools() directly on the ReactFiberReconciler() return value.
This way any renderer can choose to call it (or not call it).

Also moves us closer to not using cross-package /src/ imports.

[gaearon]

@iamdustan Can you verify this solves your use case?

[bvaughn]

Nice 👍

[bvaughn]

Are the wrapper functions necessary? Couldn't we just do...

    injectIntoDevTools(devToolsConfig: DevToolsConfig<I, TI>): boolean {
      const {findFiberByHostInstance} = devToolsConfig;
      const findFiberByHostInstanceImpl = findFiberByHostInstance
        ? findFiberByHostInstance
        : () => null;

      return ReactFiberDevToolsHook.injectInternals({
        ...devToolsConfig,
        findHostInstanceByFiber: findHostInstance,
        findFiberByHostInstance: findFiberByHostInstanceImpl,
      });
    },

[gaearon]

That would work too. I don't feel strongly as they're only called once.

[gaearon]

I tried using your approach, but it seems like Flow fails to catch some issues for some reason now. I'd prefer keeping it explicit so it's better typed.

[bvaughn]

Boo, Flow.

Okay. No biggie.

[iamdustan]

I missed the merge by a few minutes, but copying this build into Yomguithereal/react-blessed#60 and updating accordingly didn’t seem to work out of the box like before. I think that we need the FiberTreeReflection exposed as well 😬

[iamdustan]

👍

[iamdustan]

does the isAppActive() method no longer exist?

[gaearon]

Can you remind me where it is used?

[iamdustan]

It used to be part of the devtools config I think. https://github.com/facebook/react-devtools/blob/ef493abba0d08c0243bdb1f2183edc797d5d366c/packages/react-devtools-core/src/backend.js#L13-L18

I think the usage was so that an app could indicate whether it should take over the devtools connection or not, but not really sure. Or maybe to return false while still starting up (according to this comment)

https://github.com/facebook/react-devtools/blob/ef493abba0d08c0243bdb1f2183edc797d5d366c/packages/react-devtools-core/src/backend.js#L52-L55

[gaearon]

It's not required. I added it for RN so that multiple apps on simulator don't steal devtools from each other.

[gaearon]

Also, it's part of the options to connectToDevTools() call which sets up a websocket connection.

This is not related to the injectIntoDevTools call which injects renderer into a global variable.

[iamdustan]

I think the devtools integration used to work without this method 🤔

With this being required I think we’d also need ReactFiberTreeReflection.findCurrentHostFiber and ReactFiberTreeReflection.findCurrentHostFiberWithNoPortals also exposed.

[gaearon]

I don't think we need them. This one doesn't have to be the current one. So as long as you save the fiber on some field on the instance (or use a weakmap), you can get one back.

[bvaughn]

It still does. The implementation (below) is:

        findFiberByHostInstance(instance: I | TI): Fiber | null {
          if (!findFiberByHostInstance) {
            // Might not be implemented by the renderer.
            return null;
          }
          return findFiberByHostInstance(instance);
        },

[gaearon]

That's also true. (Though it's nice to implement if you can I guess.)

[gaearon]

but copying this build into Yomguithereal/react-blessed#60 and updating accordingly didn’t seem to work out of the box like before

By “updating accordingly” you mean...? It’s not clear what you added there :-)

It won't just work out of the box, you need to call this method. See changes I did in ReactDOM for example.

[gaearon]

If you can give me a branch that doesn't work I can take a look.

[iamdustan]

I just pushed to Yomguithereal/react-blessed#60 with the “update accordingly” bit 😄

[iamdustan]

You can test it by copying a fresh build from master in, run react-devtools in one tab and then yarn animation in another tab.

[gaearon]

I just pushed to Yomguithereal/react-blessed#60 with the “update accordingly” bit

I think you're passing a config for require('react-devtools-core').connectToDevTools(). This is not related to the injectIntoDevTools() function I added. I'm sorry these two sound similar. There's a few moving parts and I couldn't come up with better names.

The config you need to pass to connectToDevTools() to establish a websocket connection stays the same.

The config you need to pass to renderer.injectIntoDevTools() is different. You can look at changes to ReactDOM.js in this file to see an example of it. injectIntoDevTools() is roughly equivalent to injectInternals() call in the past.

[iamdustan]

👍 got it! This is working like a charm. Thanks, Dan!

[gaearon]

Sweet.