A way to get at top level rendered element

[dephiros]

Describe the Feature

At my company, we have a few uses case where we want to make assertion on the top level rendered element. Pre v7, we were able to do const element = view.container.children?.[0] || null. Is there a way to get the same result for post v7 without adding additional testID?

Trivial use cases example:

const Component = (render=true) => (render ? <View></View> : null);

// assert that component render when render is true
// assert that component render nothing when render is false

Possible Implementations

I see that we have renderer.root here. Maybe just have to expose that element

Related Issues

#477 (comment)

[klekowskim]

+1

Also it is needed when you are trying to add custom queries. In RN we can have similar things as in dom-testing: https://testing-library.com/docs/dom-testing-library/api-helpers#custom-queries

I agree that using container.parentNode or container.children.find(...) is an implementation details and should be avoided. But still in some cases it is helpful and it is not so bad option when you have in one place created custom queries for finding components in such way - you have it in one place. As an example we can look at this code: https://github.com/romgain/react-select-event/blob/master/src/index.ts#L13
This is a testing library helper for very popular react-select. When you looking at code it is really terrible going a few nodes up to find a component. But when we are the users of the API of this lib, we do not care. It should always be inline with the library. Sometimes this is the only option and it is better to have it in one place.

Basic example of custom query is instead of having in multiple tests getByTestID('my-component') you can create your custom queries: getMyComponent, getAllMyComponents etc. (which can be generated).
Another example would be when we want to narrow the the place where we are looking for, so instead of getByText we can have, getByTextInHeader(text), or getByTextInModal(container, modalNameOrId, text)

[thymikee]

we want to make assertion on the top level rendered element

What kind of assertions? Do they test something meaningful? You can always add a testID to this component if you really need access to that instance. But from the user perspective – does it make sense? Maybe this test can be deleted? It's worth to ask such questions, as there may be better ways to test. If there aren't I'm open to discuss a specific use case, explained in detail, so we can figure something out.

const element = view.container.children?.[0] || null

This code is testing implementation details unfortunately. How would the user know about children or parent elements? They don't. In the spirit of Testing Library, we shouldn't allow to easily grab it.

https://github.com/romgain/react-select-event/blob/master/src/index.ts#L13 is a counter-example of how one should test. It's not much different than using dive() from Enzyme. It makes tests fragile. Even though it's defined in one place, there may be multiple places that rely on it, and thus could break.

Basic example of custom query is instead of having in multiple tests getByTestID('my-component') you can create your custom queries: getMyComponent, getAllMyComponents etc. (which can be generated).

Like this?

function getMyComponent(component) {
  const {getByTestId} = render(component);
  return getByTestId("my-component");
}

I'm not sure why access to the container would be necessary in such case. Would love to have more details and see if we really can't solve it with existing tools.

Another example would be when we want to narrow the the place where we are looking for

That's what within helper is doing. Have you tried it?

[klekowskim]

In general you have right. I'll try different arguments:

1. Why we should not export root? This returns the same object as every query, and I can use test data id on a wrapper query, and using getByTestId I can have my root component. I'm saying that there is a workaround, but this should not be necessary.
2. As far as I understand everything under testing-library tries to have common approach and common rules. Why people from more mature react-testing-lirbrary exports baseElement and container? I think because:
   • they can use it to create a custom queries
   • it is regular object that is returned from queries, so if I can use object returned from query why I cannot use my root in the same way?

Regarding a counter-example from react-select-event. What they solve is the following problem: "how to test things from library that is not ours, and do not have accessibilityLabel on every element, and do not have testId on elements that we need to click".
Using testId is the same implementation details as e.g. going .parentNode twice in a custom query. In both cases there is some logic that can change.

That's what within helper is doing. Have you tried it?

Yes, a lot but in react-testing-library using container. How I can use within on my root component?

[thymikee]

it is regular object that is returned from queries, so if I can use object returned from query why I cannot use my root in the same way?

This is actually a good callout. The way it worked with previous implementation was the ReactTestInstance was wrapped in a NativeTestInstance which only exposed a subset of properties and methods. Which was likely better than exposing it as a whole like we do now. Maybe it wouldn't hurt so bad to expose it, but as you can see it easily leads to fragile tests 😅

[klekowskim]

By previous implementation you mean the second repo? That's true, but there was a root component available :)

I was using a lot the react-testing-library, and I know that this is really different because we do not have DOM here. We have the ReactTestInstance. It can be even wrapped, and expose limited API as far as it allows me to create a custom query (similar as for web).

I agree with the testing-library principles, but there are situations that we are forced to do things in different ways. First example is to add testId - it is not recommended but there is an option available. Second is here :)

I do not have strong arguments for exporting a root, and this is not a thing that everyone must and want to use. I would say that this is not a bad thing to export it, especially there is an easy workaround. And maybe because of this workaround this should be exported :)

[thymikee]

As long as we make the API identical to the React Testing Library, I think I'm fine. We can later wrap ReactTestInstance with some custom abstraction and release in another major update, as it would most certainly break a few usages.

[thymikee]

Together with @pvinis we found a way to expose root ReactTestInstance without making it too easy to use – through an API designed to extend default queries. Here's a rough idea: #569 (comment)

const getByTextWithType = (root) => { ... }
const customRender = render<typeof getByTextWithType>(<Comp />, {extendWithQueries: {getByTextWithType}});
const {getByTextWithType} = customRender(<Comp />)

[klekowskim]

IMHO this can be added, but together with exporting the root component :) Trying to hide the element that is needed to work with custom queries, and it is also returned from the query is just not logical. I can prepare workaround to have this root container either using wrapper option or suggested by you extendWithQueries.

1. Workaround using new extendWithQueries:

const getRoot = (container) => { return container }
const {getRoot} = render(<Comp />, {extendWithQueries: {getByTextWithType}});
const root = getRoot()

2. Workaround using wrapper

function render(component, options) {
const result = render(component, {...options, wrapper: <View testId='root'>{children}</View>}});
const root = getByTestId('root');
return {...result, root };
}

I believe that either we agree that root/container and custom queries are allowed or not. I understand that the goal is to prevent testing internal things and creating more stable tests. But not exporting the root is not doing that. Additionally there are exported queries that brakes the rules.

In react-testing-library you have access to the DOM of rendered components. With that you can of course use .parentNode, children or check the attributes. But is not recommended. The similar situation is here and not exporting root does not change anything, besides forcing users to do workarounds :)

[pvinis]

I see #567 exists, which seems like a good way to expose root, and I just made #573 which can work with the other PR or it can override it and use a custom getRoot query. Either way, I'd love some feedback and maybe a bit of help with the types. Thanks all.

[thymikee]

Closed via #567