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

ide · 2019-05-10T01:02:16Z

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.

sizebot · 2019-05-10T01:13:38Z

	Warnings
⚠️	Could not find build artifacts for base commit: `101901d`

Generated by 🚫 dangerJS

ide · 2019-05-13T17:19:30Z

@cpojer Since the RN side of this work in facebook/react-native#24807 has landed (thank you!), the renderers generated by this commit should be safe to sync out to RN whenever the next sync happens. Because we want to migrate away from Haste in RN 0.61, this means this commit needs to land and a React renderer sync needs to happen sometime before the next release and ideally with enough time to land & smoke test the follow-up commits that update the Jest/Flow/etc configs.

sebmarkbage

lgtm.

(I think we can get rid of FabricUIManager and just inline references to the global since we do that elsewhere now.)

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).

ide · 2019-05-13T20:52:41Z

Added a commit to replace all the FabricUIManager imports with global nativeFabricUIManager references.

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).

…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.

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).

cpojer

Let's do it.

sebmarkbage · 2019-05-23T16:21:46Z

Who is doing the sync of these changes to RN?

cpojer · 2019-05-23T16:32:44Z

If there is a way to sync only the changes from this PR then I’m happy to do it next week. Otherwise I was gonna wait until the next time we need to do a sync.

ide · 2019-05-23T18:18:08Z

@cpojer I think it wouldn't be too hard to check out the copy of React that was used for the last sync, cherry-pick these changes, and then copy them over to the RN repo. Here is a PR: facebook/react-native#25010.

This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end.

This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

…sion This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

…sion (#15786) This commit is a follow-up to #15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

…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).

…sion (facebook#15786) This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

…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).

…sion (facebook#15786) This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

Summary: This isn't a full sync. It only includes cherry-picked commits for Fresh, as well as previously cherry-picked commits during last two syncs. Changes that are already synced: * facebook/react#15604 * facebook/react#15786 * facebook/react#15802 New changes: * gaearon/react@5be064b * gaearon/react@6b6d8fa * gaearon/react@92bcf2a * gaearon/react@7b4c2f7 * gaearon/react@39a3f2f * gaearon/react@cabdebb * gaearon/react@a277671 * gaearon/react@86aad18 * gaearon/react@25b3e07 * gaearon/react@ffee295 * gaearon/react@a817bc5 * gaearon/react@abb2f0f * gaearon/react@2e20d9e * gaearon/react@5d35024 * gaearon/react@6628573 * gaearon/react@5815bff **This might look like a lot but note that very few of these actually touch the renderer code.** Most affect integration tests and `react-refresh` package which **has already been synced separately**. So this is about getting in a few renderer changes that were included in those commits. That's very self-contained. The renderer source diff is this: https://gist.github.com/gaearon/ddbda5845b4dba0d9915e2ed7f7b11e2. You can also see that in prod bundles below there's only one meaningful change (`type` used instead of `elementType`) which in that case is equivalent. Reviewed By: rickhanlonii Differential Revision: D15898205 fbshipit-source-id: 19619f4ff01f24765613f19e826b0199485d81bb

…sion (facebook#15786) This commit is a follow-up to facebook#15604, which explains more of the rationale behind moving React Native to path-based imports and the work needed in the React repository. In that linked PR, the generated renderers were updated but not the shims; this commit updates the shims. The problem is that FB needs a different copy of the built renderers than the OSS versions so we need a way for FB code to import different modules than in OSS. This was previously done with Haste, but with the removal of Haste from RN, we need another mechanism. Talking with cpojer, we are using a `.fb.js` extension that Metro can be configured to prioritize over `.js`. This commit generates FB's renderers with the `.fb.js` extension and OSS renderers with just `.js`. This way, FB can internally configure Metro to use the `.fb.js` implementations and OSS will use the `.js` ones, letting us swap out which implementation gets bundled. Test Plan: Generated the renderers and shims with `yarn build` and then verified that the generated shims don't contain any Haste-style imports. Copied the renderers and shims into RN manually and launched the RNTester app to verify it loads end-to-end. Added `.fb.js` to the extensions in `metro.config.js` and verified that the FB-specific bundles loaded.

Summary: This isn't a full sync. It only includes cherry-picked commits for Fresh, as well as previously cherry-picked commits during last two syncs. Changes that are already synced: * facebook/react#15604 * facebook/react#15786 * facebook/react#15802 New changes: * gaearon/react@5be064b * gaearon/react@6b6d8fa * gaearon/react@92bcf2a * gaearon/react@7b4c2f7 * gaearon/react@39a3f2f * gaearon/react@cabdebb * gaearon/react@a277671 * gaearon/react@86aad18 * gaearon/react@25b3e07 * gaearon/react@ffee295 * gaearon/react@a817bc5 * gaearon/react@abb2f0f * gaearon/react@2e20d9e * gaearon/react@5d35024 * gaearon/react@6628573 * gaearon/react@5815bff **This might look like a lot but note that very few of these actually touch the renderer code.** Most affect integration tests and `react-refresh` package which **has already been synced separately**. So this is about getting in a few renderer changes that were included in those commits. That's very self-contained. The renderer source diff is this: https://gist.github.com/gaearon/ddbda5845b4dba0d9915e2ed7f7b11e2. You can also see that in prod bundles below there's only one meaningful change (`type` used instead of `elementType`) which in that case is equivalent. Reviewed By: rickhanlonii Differential Revision: D15898205 fbshipit-source-id: 19619f4ff01f24765613f19e826b0199485d81bb

facebook-github-bot added the CLA Signed label May 10, 2019

This was referenced May 10, 2019

[standards] Umbrella issue to track migration to standard path-based requires (remove Haste) facebook/react-native#24316

Closed

[standards] Migrate React renderers from Haste to path-based imports facebook/react-native#24770

Closed

ide force-pushed the hasteless-react-native branch 2 times, most recently from edb8583 to 10eab79 Compare May 10, 2019 05:14

cpojer requested a review from sebmarkbage May 13, 2019 14:51

sebmarkbage approved these changes May 13, 2019

View reviewed changes

ide force-pushed the hasteless-react-native branch from 171cdb9 to 5438d58 Compare May 13, 2019 21:03

ide added 2 commits May 22, 2019 13:53

ide force-pushed the hasteless-react-native branch from 5438d58 to 8b659a2 Compare May 22, 2019 21:36

cpojer approved these changes May 23, 2019

View reviewed changes

cpojer merged commit 61f6224 into facebook:master May 23, 2019

ide deleted the hasteless-react-native branch May 23, 2019 17:24

ide mentioned this pull request May 31, 2019

[react-native] Rewrite Haste imports in RN shims and add .fb.js extension #15786

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

ide commented May 10, 2019

Uh oh!

sizebot commented May 10, 2019 •

edited

Loading

Uh oh!

ide commented May 13, 2019

Uh oh!

sebmarkbage left a comment

Uh oh!

ide commented May 13, 2019

Uh oh!

cpojer left a comment

Uh oh!

sebmarkbage commented May 23, 2019

Uh oh!

cpojer commented May 23, 2019

Uh oh!

ide commented May 23, 2019

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

ide commented May 10, 2019

Uh oh!

sizebot commented May 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ide commented May 13, 2019

Uh oh!

sebmarkbage left a comment

Choose a reason for hiding this comment

Uh oh!

ide commented May 13, 2019

Uh oh!

cpojer left a comment

Choose a reason for hiding this comment

Uh oh!

sebmarkbage commented May 23, 2019

Uh oh!

cpojer commented May 23, 2019

Uh oh!

ide commented May 23, 2019

Uh oh!

Uh oh!

sizebot commented May 10, 2019 •

edited

Loading