Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing for Platform Traits and Quirks instead of OS #293

Open
NickGerleman opened this issue Nov 16, 2020 · 1 comment
Open

Testing for Platform Traits and Quirks instead of OS #293

NickGerleman opened this issue Nov 16, 2020 · 1 comment
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject

Comments

@NickGerleman
Copy link

Introduction

Different platforms running react-native have various implementation quirks inherent to the platform itself. This can lead to internals of react-native testing for Platform.OS to make branching decisions based off of implementation. This style of OS testing often causes issues for out-of-tree platforms, which need to fork private RN implementation to add themselves as a branch.

In many of these cases, the condition we're interested in can be expressed as a trait of the platform. E.g. in facebook/react-native#30374 we special-case Android, due to keyboard events not firing in certain cases on the platform. Instead of testing for Platform.OS === 'android', it is conceivable this limitation could instead be declared by the RN android platform itself. E.g. an API like Platform.quirks.keyboardEventsSometimesUnavailable. The same mechanism could be used to declare other interesting traits, like Platform.supportsMultipleViewports for whether usage of the Dimensions API is unsafe.

This proposal would introduce finer-grained Platform constants as a mechanism to allow shared code to adapt to Platform-specific behavior, while remaining friendly to out-of-tree platforms.

Details

Implementation could extend off of the existing PlatformConstants module, allowing native code for a platform to describe its abilities and limitations. Platforms should declare values for each of the traits, providing a fallback value for when using an older native module (both FB and Microsoft internally may use out-of-sync native and JS).

To avoid crowding the top-level Platform API, more fine-grained traits may be expressed behind a Quirks object, describing specific behavior

Discussion points

  • Decentralization of trait declaration: This approach makes RN core oblivious to what platforms have specific traits. This allows code in core to be somewhat generic to specific platforms, but also imposes risk with maintenance. E.g. it is harder to reason about who is relying on a quirk, and whether it it still applicable. This is somewhat eased by the limited number of popular out-of-tree platforms.

  • Global vs Local Traits: This approach creates a hub aware of the rest of the system. This has both advantages and disadvantages. Grouping traits related to all systems in a single place helps out-of-tree platforms to discover and accurately respond to them. Grouping them has the potential to create a bit of a dumping if widely adopted though, making it harder to maintain in relation to the code effected.

  • Public vs private: Some finer-grained platform information may be useful to library writers wanting to support multiple platforms. Quirks being tied to consumption in core makes the interface less likely to be stable however. A possible separation could be that quirks are private, but more fundamental traits are public.

  • Single platform traits vs OS testing: Platform quirks may sometimes belong to a single OS, in which case handling its behavior in core still feels like coupling it to a specific platform. I.e. it acts as a more abstracted Platform.OS check for an out-of-tree platform. It does architecturally let core be oblivious of the existence of the platform, but the actual maintenance in core would look very similar if untested.

  • Adoption: For the paradigm shift to achieve its goals, it would need to be followed across implementers of new library code. To do this it seems like we would need a critical mass of traits and trait testing. Staging this seems tricky.

@kelset kelset added the 🗣 Discussion This label identifies an ongoing discussion on a subject label Nov 16, 2020
@kevinvangelder
Copy link

IMO, this simply punts the problem. It's a clever solution, however instead I think the focus should be on extracting Android/iOS (as has been previously discussed) so that all platforms are on equal footing and can simply notify RN of it's presence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗣 Discussion This label identifies an ongoing discussion on a subject
Projects
None yet
Development

No branches or pull requests

3 participants