-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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.
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', ...], } ```
cpojer
approved these changes
Jun 3, 2019
There was a problem hiding this 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.
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 inmetro.config.js
and verified that the FB-specific bundles loaded.