[DevTools] find best renderer when inspecting

[mondaychen]

Summary

resolves #24539

When inspecting with DevTools, originally we use the first renderer where we can found a fiber with inspected element. However, if two or more renderers are nested (see the example in the issue #24539), this might cause a mismatch.
In this PR, we now always go through all renderers until we find an exact match (where the inspected element is exactly the same stateNode linked to the fiber). If we cannot find an exact match, we fallback to the original logic and return a renderer where we find a non-exact matching fiber.

How did you test this change?

I built the extension and tested with the same URL the issue author provided. You can see the inspecting tool can now correctly select the "MicroFeRoot" node (and the element inside)

Screen.Recording.2022-06-02.at.5.23.38.PM.mov

[lunaruan]

This probably should be in the backend/agent file instead of the utils because you're using two for loops right now and if you put the code from getBestMatchingRendererInterface into the agent you would only need one

ex.

let renderer = this._rendererInterfaces[0];
for (const rendererID in this._rendererInterfaces) {
  const currentRenderer = ...;
  const fiber = currentRenderer.getFiberForNative(node, false);
  if (fiber !== null && fiber.stateNode === node) {
   renderer = currentRenderer;
    break;
  }
  ...
}
...

[mondaychen]

The first for-loop is only there for proper typing. Maybe I should refactor the types of this._rendererInterfaces?
It's indeed easier to do everything agent, but how about the use of the util function in Overlay?

[mondaychen]

I updated the file with better typing d0346ff so that I can use values() instead of for-loop 9a7e0be

Does it make sense?

[lunaruan]

I think this PR might break the legacy renderer. Could you take a look? 😊

[lunaruan]

I understand why you're doing this (following prior precedent) but I personally think this would be more readable if GetFiberForNative was inlined. What do you think?

[lunaruan]

fiber !== null

[lunaruan]

Right now you're setting bestMatch every renderer where fiber doesn't equal null. What do you think about keeping the original behavior if the exact fiber isn't found (ie. returning the first matching renderer)?

[lunaruan]

Does this not require an ID?

[lunaruan]

Maybe instead of making getDisplayNameForNode you can use getBestMatchingRendererInterface, getFiberIDForNative, and gwetDisplayNameForFiberID?

[mondaychen]

@lunaruan updated 😃

[lunaruan]

Should we wrap this around a try/catch like before?

[mondaychen]

I'll add it back for safety, but it looks pretty vague what exactly it's trying to catch. The comment says it's for old react, and commit itself is pretty old too. Do we know in which version this might cause an error, so we can get rid of it one day?

[lunaruan]

That's a good question. I'm not sure. We're currently supporting React 16, which was released 5 years ago, so we're supporting pretty far back. I'm not sure exactly which version causes an error, but if you want to find out and add a comment that'd be great 😊

[lunaruan]

LGTM!