feature: make all (safe) queries return host components

[mdjastrzebski]

Describe the Feature

Currently different types of queries can return either host or composite elements:

Queries returning host components (typeof node.type === 'string'):

• testId: host, matched by normalised testID prop
• role: host, matched by accessibilityRole prop
• hintText: host, matched by accessibilityHint prop
• labelText: host, matched by accessibilityLabel prop
• a11yState: host, matched by accessibilityState prop
• a11yValue: host, matched by accessibilityValue prop

Queries returning composite components:

• text: composite Text from RN, matched by normalised and flattened children prop
• displayValue: composite TextInput from RN, matched by normalised value or defaultValue prop
• placeholderText: composite TextInput from RN, matched by normalised placeholder prop

Queries returning either host or composite components:

• UNSAFE_byType: either host or composite, matched by element.type which can be either string or component type
• UNSAFE_byProps: either host or composite, matched by element.props overlap

The goal of this feature request is to make text, displayValue & placeholderText return host components. This is probably a breaking change for our users so it should warrant a major release.

Possible Implementations

Use component-tree.ts functions from #1128 to get single host child of found composite Text and TextInput components from React Native.

Related Issues

#1092

@thymikee @AugustinLF @pierrezimmermannbam wdyt?

[pierrezimmermannbam]

I think this would be a really good change as all queries would now have the same behavior.

Regarding the implementation, I was initially thinking about changing the filtered type in all of the three queries by the host type (for instance using filterNodeByType with type 'Text' instead of Text component required from react-native). This works because the props children, placeholder and value are both on the host and the composite component and this way the queries really look for host components instead of looking for a composite component and returning its child, which I think is a bit better. Also the required changes would be really minimal

It's arguably a bit more dangerous than the approach you suggested and there may be some edge cases that aren't covered by tests and that I can't think of (at least for text queries, the two others being relatively simple and thus pretty safe to change this way), so I'm not entirely sure what's the best way to proceed here

[thymikee]

👍🏼

[AugustinLF]

Overall I'm all for always returning the same type of component. From a user-land perspective, what is the difference between a host and composite component (apart from node.type)? What types of tests we would break?

Small note, we have a few PRs that include some nice improvement (react 18 + accessibility) and none of them are breaking changes, so if we can get those in before such change, that'd be nice.

[mdjastrzebski]

re difference between host and composite components:

• host components is what actually is visible to the user as they represent native views, text labels & inputs, buttons, etc
• you can think of host components as HTML tags like div, span, h1, etc of React Native development
• composite components (function and class components) exist only as developer abstraction and are not directly rendered to the user, only their host children are visible.
• in some cases like TextInput, Text, View from React Native there is a close correspondence between these two, as you usually import composite component that render a single host component underneath
• that single child can be named the same as composite component (e.g. View, Text, TextInput) or differently (SafeAreaView vs RCTSafeAreaView)
• in other cases like TouchableOpacity the rendered host view is actually a View with properly assigned props.
• the props as described in RN docs usually apply to composite components, however in most cases they are passed down to native views

re priorities: no rushing with this idea, I wanted to gather your feedback and see if there are any potential problems that I did not notice myself. All of the PRs you mentioned will surely get merged before this gets implemented.

[AugustinLF]

Thanks a lot for the detailed explanation, helps a lot! Definitely something we should add to the doc when doing this change.

I think it would be good, but we should be really careful about the release, and including that some form of rc release, or (if possible) behind a flag.

[pierrezimmermannbam]

It should have relatively few influence on user tests but I guess it depends on how you use the api. FireEvent should continue to work as before since it reaches out to the parent when it doesn't find the prop on the element. The main issue would be that the props of the returned element would be different in some cases (onClick vs onPress for a Touchable for instance, although touchable can only be queried directly by testID which already returns host components). But even so the main usage of accessing directly the returned element's props would be to check style I guess and this should work fine also since the prop is shared between the host and the composite element. I've applied this change and tried to run the tests in the application I'm currently working on and none broke.

The change could be very confusing though and quite hard to debug so a migration guide with a detailed explanation of the change and its involvements would probably be necessary

[mdjastrzebski]

We could add a legacy: true option for affected queries (*ByText, etc), so that the users are not forced to migrate everything at once. but just can add that option and continue as is. Giving them more time to migrate things.

[AugustinLF]

The change could be very confusing though and quite hard to debug so a migration guide with a detailed explanation of the change and its involvements would probably be necessary

Yeah exactly. The benefit of having it as an option, is that it makes it very easy to pinpoint that this specific feature is the problem.

Once there'll be an open PR I'll also try to apply it to my codebase to see the impact, people have been going pretty wild in with tests so I'd expect it to be a good candidate.

[mdjastrzebski]

I plan to submit such PR after we get #1139 merged.