Container API is broken

[mdjastrzebski]

Our container API, tries to mimic RTLs API of the same name, but does actually something else.

One user of Jest Native posted following piece of test:

const { container } = render(<Test />);
expect(container).toBeEmptyElement();

The main problem with container API is that it returns root of Test Renderer's ReactTestInstance tree: composite Test element, while the user is expecting behavior as if the returned element was root host element. All our queries are (soon) returning only host elements, but this one exposes a composite one.

Additionally, in RTL, container is a DOM element where test render results are attached, i.e. they are the DOM parent of root element, where in our case the is no concept of parent of not rendered output. The only common thing here is that both ours and RTL's queries are bound to container element.

That leaves us with a number of options:

1. Leave as is, but document that this is dangerous api (or rename to UNSAFE_container - breaking)
2. Make container reference root host element (breaking change), perhaps under different name like root (non-breaking).
3. Make container reference an host View that would wrap users JSX elements. This will be most true to life with RTL, but toJSON / debug will now print this View by default, breaking all snapshot tests by wrapping them in View. This could be mitigated by making toJSON() return only children of that View.

Wdyt @thymikee @AugustinLF @pierrezimmermannbam @MattAgn?

[pierrezimmermannbam]

What I lack to fully understand the issue here is how this API is used by users, both RTL and RNTL users. I don't use it personally and I don't see use cases for it, have you any ideas on this?

If we feel like it serves no purpose another option would be to remove this API, it would be a change from RTL's API but maybe it's better than having it but with a different behavior from RTL's behavior.

Otherwise I think I like the second option most, I'd guess it shouldn't have an important impact on user tests and I don't really like the idea of adding an additional View

[mdjastrzebski]

The test I've put as an example, and which I think might be a valid patter for component test is that you want to access root host component without giving it testId and using getByTestId to find it.
The original GH issue also mention this use case: #566

[pierrezimmermannbam]

Oh ok I see. Then I agree it should be kept and modified. In that case I like option 2 best, mainly because I'd find it weird that the root is not null when I'm rendering nothing and the breaking change would also be kinda confusing. That being said I'm not a regular user of RTL so I'm not used to their API and having a logic similar to theirs has its advantages, I'm just not sure it makes sense in this particular case

[mdjastrzebski]

container in RTL is not root element, but parent of root element. The even explicitly mention how to get root element:

Tip: To get the root element of your rendered element, use container.firstChild.

That being said, since our tree is mix of composite and host elements, using first child is not advised, as it will probably result in some random composite element. Therefore, exposing host root seems like a reasonable solution, although not 100% compatible with RTL as they do not have such prop. However, React Test Renderer has such prop (but usually returning composite element).

[thymikee]

It's worth to revisit the discussion we had when adding this API: #567. The way RNTL and RTL render components in a tree are fundamentally different, as we don't use React.createRoot. And I think it's OK to keep it this way and not emulate the compatibility with extra wrapper View for the sake of compatibility only, unless it's only for the container. That being said, picking the container name for our current functionality was unfortunate imho.

I think we should consider exposing the ReactTestInstance that's currently under container as UNSAFE_root or such (it's an escape hatch to get to the react test renderer full potential, but then you're on your own). And at the same time either slowly deprecate the container itself, or make it work closer to RTL by having it return an emulated root host View that wouldn't affect other queries.

[mdjastrzebski]

Based on this discussion, I've prepared a concrete proposal for how to fix issues with container API: #1296