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

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

Merged
merged 1 commit into from
Jun 3, 2019

Conversation

ide
Copy link
Contributor

@ide ide commented May 31, 2019

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.

…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.
@sizebot
Copy link

sizebot commented May 31, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS

ide added a commit to expo/react-native that referenced this pull request May 31, 2019
…ts instead

This commit uses the same base commit of React that's currently in RN (React ec6691a68716bc59291746fc62f374a56fb435c9) plus a commit in the React repo that removes Haste-style imports from the renderer and a commit to make the shims import from `implementations` (facebook/react#15786).

I built React in the React repo with `yarn build` and copied over the `oss` directory into one called `implementations` and removed the `*.fb.js` files.

Test plan: run unit tests, RNTester, CI. Also verified that we can make Metro pick up `.fb.js` files with:

```js
resolver: {
  sourceExts: ['fb.js', 'js', 'json', ...],
}
```
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.

Awesome, let me cherry-pick and update RN at FB with this.

@cpojer cpojer merged commit 07da821 into facebook:master Jun 3, 2019
@ide ide deleted the react-native-hasteless-shims branch June 3, 2019 16:49
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 5, 2019
…ts instead (#25100)

Summary:
**This is a manual React sync to replace Haste names with paths so that the removal of Haste is not blocked on a normal React sync. This is a one-time special case; in the future, React will generate renderers that use paths instead of Haste.**

This commit uses the same base commit of React that's currently in RN (React ec6691a68716bc59291746fc62f374a56fb435c9) plus a commit in the React repo that removes Haste-style imports from the renderer (61f62246c8cfb76a4a19d1661eeaa5822ec37b36) and a commit to make the shims import from `implementations` (facebook/react#15786).

I built React in the React repo with `yarn build` and copied over the `oss` directory into one called `implementations` and removed the `*.fb.js` files.

## Changelog

[General] [Changed] - Sync React with Haste-style imports rewritten to use path-based imports instead
Pull Request resolved: #25100

Reviewed By: yungsters

Differential Revision: D15575646

Pulled By: cpojer

fbshipit-source-id: adf25f9826b71729043b65ba1afd20d14d8c19c4
gaearon pushed a commit to gaearon/react that referenced this pull request Jun 11, 2019
…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.
gaearon pushed a commit to gaearon/react that referenced this pull request Jun 19, 2019
…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.
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 19, 2019
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
rickhanlonii pushed a commit to rickhanlonii/react that referenced this pull request Jun 25, 2019
…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.
vovkasm pushed a commit to vovkasm/react-native that referenced this pull request Aug 7, 2019
…ts instead (facebook#25100)

Summary:
**This is a manual React sync to replace Haste names with paths so that the removal of Haste is not blocked on a normal React sync. This is a one-time special case; in the future, React will generate renderers that use paths instead of Haste.**

This commit uses the same base commit of React that's currently in RN (React ec6691a68716bc59291746fc62f374a56fb435c9) plus a commit in the React repo that removes Haste-style imports from the renderer (61f62246c8cfb76a4a19d1661eeaa5822ec37b36) and a commit to make the shims import from `implementations` (facebook/react#15786).

I built React in the React repo with `yarn build` and copied over the `oss` directory into one called `implementations` and removed the `*.fb.js` files.

## Changelog

[General] [Changed] - Sync React with Haste-style imports rewritten to use path-based imports instead
Pull Request resolved: facebook#25100

Reviewed By: yungsters

Differential Revision: D15575646

Pulled By: cpojer

fbshipit-source-id: adf25f9826b71729043b65ba1afd20d14d8c19c4
(cherry picked from commit 69d1ed7)
aleclarson pushed a commit to alloc/react-native-macos that referenced this pull request Nov 2, 2019
…ts instead (#25100)

Summary:
**This is a manual React sync to replace Haste names with paths so that the removal of Haste is not blocked on a normal React sync. This is a one-time special case; in the future, React will generate renderers that use paths instead of Haste.**

This commit uses the same base commit of React that's currently in RN (React ec6691a68716bc59291746fc62f374a56fb435c9) plus a commit in the React repo that removes Haste-style imports from the renderer (61f62246c8cfb76a4a19d1661eeaa5822ec37b36) and a commit to make the shims import from `implementations` (facebook/react#15786).

I built React in the React repo with `yarn build` and copied over the `oss` directory into one called `implementations` and removed the `*.fb.js` files.

## Changelog

[General] [Changed] - Sync React with Haste-style imports rewritten to use path-based imports instead
Pull Request resolved: facebook/react-native#25100

Reviewed By: yungsters

Differential Revision: D15575646

Pulled By: cpojer

fbshipit-source-id: adf25f9826b71729043b65ba1afd20d14d8c19c4
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
…ts instead (facebook#25100)

Summary:
**This is a manual React sync to replace Haste names with paths so that the removal of Haste is not blocked on a normal React sync. This is a one-time special case; in the future, React will generate renderers that use paths instead of Haste.**

This commit uses the same base commit of React that's currently in RN (React ec6691a68716bc59291746fc62f374a56fb435c9) plus a commit in the React repo that removes Haste-style imports from the renderer (61f62246c8cfb76a4a19d1661eeaa5822ec37b36) and a commit to make the shims import from `implementations` (facebook/react#15786).

I built React in the React repo with `yarn build` and copied over the `oss` directory into one called `implementations` and removed the `*.fb.js` files.

## Changelog

[General] [Changed] - Sync React with Haste-style imports rewritten to use path-based imports instead
Pull Request resolved: facebook#25100

Reviewed By: yungsters

Differential Revision: D15575646

Pulled By: cpojer

fbshipit-source-id: adf25f9826b71729043b65ba1afd20d14d8c19c4
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants