Add ReactIntrospection #6425
Add ReactIntrospection #6425
Conversation
|
Overall I’m really unhappy with how tests mock the ID map. They currently break some basic assumptions about it. I’ll see if extracting something like |
|
To get rid of mocking, I changed this PR to add |
|
@gaearon updated the pull request. |
This is another change extracted from #6046 and covered by tests. Here, we add `ReactIntrospection` which will be used by the new ReactPerf, as well as (eventually) by React DevTools and third-party React tooling. It abstracts away the implementation details of internal instances so those tools won't rely on these details. Functions in this module take and return internal instances directly for the ease of testing. We will add a separate `ReactDebugInstrumentation` facade later that takes and returns IDs introduced in #6389. The plan is to not expose the internal instances to React DevTools and ReactPerf, but to only expose IDs.
| if (element == null) { | ||
| name = '#empty'; | ||
| } else if (typeof element === 'string' || typeof element === 'number') { | ||
| name = '#text'; |
There was a problem hiding this comment.
Alternatively I can put these into getName() of ReactDOMEmptyComponent and ReactDOMTextComponent. I’m not sure which way would be preferable. This seems like a better catch-all for other renders but maybe this doesn’t matter?
|
One thing that slightly concerns me is there’s a ton of code in tests for such a small file. My experience so far was that those functions are very fragile and it’s easy to return some nonsense or even get a null reference if you’re not careful changing them. This is especially the case for situations when we want to have special behavior for something like React ART. (Which might actually not work with this particular PR but I’ll add support later down the road.) I wanted to ensure that if we change, for example, how renderered components are stored on the native instance, start “inlining” functional components, begin merging text nodes, etc, ReactPerf and React DevTools don’t suddenly start to behave unexpectedly or fail. |
|
@gaearon updated the pull request. |
|
Ugh. I think a better alternative is to replace these unit tests with something more like integration tests where I just mount a tree, recursively aggregate all possible info, and compare the real aggregation to my expectations. I would be able to mix different kinds of components there, so the test would be much less lines but cover the same code. This way we also wouldn’t rely on any implementation details we rely on right now in tests. This would be similar to what |
This is another change extracted from #6046 and covered by tests.
Here, we add
ReactIntrospectionwhich will be used by the new ReactPerf, as well as (eventually) by React DevTools and third-party React tooling. It abstracts away the implementation details of internal instances so those tools won't rely on these details.Functions in this module take and return internal instances directly for the ease of testing. We will add a separate
ReactDebugInstrumentationfacade later that takes and returns IDs introduced in #6389. The plan is to not expose the internal instances to React DevTools and ReactPerf, but to only expose IDs.