Skip to content

[standards] ReactPrivate, an explicit interface between the renderer and RN #24782

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

Closed
wants to merge 2 commits into from

Conversation

ide
Copy link
Contributor

@ide ide commented May 9, 2019

Summary

This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (ReactNativePrivateInterface) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

Changelog

[General] [Changed] - Add a private interface (do not use) between the renderer and RN

Test Plan

Run unit tests, CI. This commit should be safe since it just introduces new modules.

Also tested with newly generated renderers (not in this commit; needs to happen in the React repo) that use ReactPrivate instead of Haste and verified that RNTester loads and that unit tests pass.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

@ide ide force-pushed the standard-paths-renderer branch from 871bfa6 to 4b03978 Compare May 9, 2019 18:30
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner labels May 9, 2019
ide added a commit to ide/react that referenced this pull request May 10, 2019
…derer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
ide added a commit to ide/react that referenced this pull request May 10, 2019
…derer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
@ide ide force-pushed the standard-paths-renderer branch from 4b03978 to 5fc65c7 Compare May 10, 2019 03:51
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

ide added a commit to ide/react that referenced this pull request May 10, 2019
…derer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
@ide ide requested a review from cpojer May 10, 2019 17:56
@ide ide force-pushed the standard-paths-renderer branch from 622fcb2 to 49fe18b Compare May 10, 2019 18:52
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • flow found some issues.

ide added 2 commits May 13, 2019 17:26
…and RN

This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

Test Plan: Run unit tests, CI. This commit should be safe since it just introduces new modules.

Also tested with newly generated renderers (not in this commit; needs to happen in the React repo) that use ReactPrivate instead of Haste and verified that RNTester loads and that unit tests pass.
The ReactPrivate modules are the new forwarding interface between RN and the renderers generated from the React repository. These `lib` files may have been unused since `InitializeJavaScriptAppEngine` is not referenced anywhere in the RN and React repositories.
@ide ide force-pushed the standard-paths-renderer branch from 49fe18b to 67eb355 Compare May 14, 2019 00:27
@ide
Copy link
Contributor Author

ide commented May 17, 2019

@cpojer -- This is ready for review (tests passing, conforms to the interface Sebastian suggested on the React PR).

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, let's try this!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @ide in 9cd8825.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 20, 2019
@ide ide deleted the standard-paths-renderer branch May 22, 2019 19:02
ide added a commit to ide/react that referenced this pull request May 22, 2019
…derer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.
cpojer pushed a commit to facebook/react that referenced this pull request May 23, 2019
…derer (#15604)

* [react-native] Use path-based imports instead of Haste for the RN renderer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.

* Access natively defined "nativeFabricUIManager" instead of importing it

Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in #15604 (review) for more context).
gaearon pushed a commit to gaearon/react that referenced this pull request Jun 11, 2019
…derer (facebook#15604)

* [react-native] Use path-based imports instead of Haste for the RN renderer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.

* Access natively defined "nativeFabricUIManager" instead of importing it

Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in facebook#15604 (review) for more context).
gaearon pushed a commit to gaearon/react that referenced this pull request Jun 19, 2019
…derer (facebook#15604)

* [react-native] Use path-based imports instead of Haste for the RN renderer

To move React Native to standard path-based imports instead of Haste, the RN renderer that is generated from the code in this repo needs to use path-based imports as well since the generated code is vendored by RN. This commit makes it so the interface between the generated renderers and RN does not rely on Haste and instead uses a private interface explicitly defined by RN. This inverts control of the abstraction so that RN decides the internals to export rather than React deciding what to import.

On RN's side, a new module named `react-native/Libraries/ReactPrivate/ReactNativePrivateInterface` explicitly exports the modules used by the renderers in this repo. (There is also a private module for InitializeCore so that we can import it just for the side effects.) On React's side, the various renderer modules access RN internals through the explicit private interface.

The Rollup configuration becomes slimmer since the only external package is now `react-native`, and the individual modules are instead listed out in `ReactNativePrivateInterface`.

Task description: facebook/react-native#24770
Sister RN PR (needs to land before this one): facebook/react-native#24782

Test Plan: Ran unit tests and Flow in this repo. Generated the renderers and manually copied them over to the RN repo. Ran the RN tests and launched the RNTester app.

* Access natively defined "nativeFabricUIManager" instead of importing it

Some places in the Fabric renderers access `nativeFabricUIManager` (a natively defined global) instead of importing UIManager. While this is coupling across repos that depends on the timing of events, it is necessary until we have a way to defer top-level imports to run after `nativeFabricUIManager` is defined. So for consistency we use `nativeFabricUIManager` everywhere (see the comment in facebook#15604 (review) for more context).
vovkasm pushed a commit to vovkasm/react-native that referenced this pull request Aug 7, 2019
…book#24782)

Summary:
This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

## Changelog

[General] [Changed] - Add a private interface (do not use) between the renderer and RN
Pull Request resolved: facebook#24782

Differential Revision: D15413477

Pulled By: cpojer

fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
(cherry picked from commit 9cd8825)
aleclarson pushed a commit to alloc/react-native-macos that referenced this pull request Nov 2, 2019
Summary:
This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook/react-native#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

## Changelog

[General] [Changed] - Add a private interface (do not use) between the renderer and RN
Pull Request resolved: facebook/react-native#24782

Differential Revision: D15413477

Pulled By: cpojer

fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
aleclarson pushed a commit to alloc/react-native-macos that referenced this pull request Nov 2, 2019
Summary:
This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook/react-native#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

## Changelog

[General] [Changed] - Add a private interface (do not use) between the renderer and RN
Pull Request resolved: facebook/react-native#24782

Differential Revision: D15413477

Pulled By: cpojer

fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
…book#24782)

Summary:
This introduces a new library named "ReactPrivate" that defines an explicit interface between the React renderers generated by the React repo and the code within RN. Previously, the React renderers would reach into RN internals via Haste wormholes. With this commit, there is now an explicit module (`ReactNativePrivateInterface`) that the renderers use to access RN internals.

Motivation: The main goal is to move one step closer to turning off Haste for RN (facebook#24316). Since the generated renderers currently use Haste, this commit sets the foundation for giving them a path-based interface to access RN internals.

Additionally, this approach inverts abstraction control since RN needs to intentionally export its internals via the private interface instead of React reaching in via Haste.

There will also need to be a corresponding commit to the React repo to make the renderers use this new interface. This RN commit needs to land before the React commit.

## Changelog

[General] [Changed] - Add a private interface (do not use) between the renderer and RN
Pull Request resolved: facebook#24782

Differential Revision: D15413477

Pulled By: cpojer

fbshipit-source-id: 3766ad4cf129fad0c82f0ddc7a485a4ba313b2c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Expo Partner: Expo Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants