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

fix how @babel/register is used and update it #1371

Closed
wants to merge 1 commit into from

Conversation

vzaidman
Copy link
Contributor

@vzaidman vzaidman commented Oct 11, 2024

Summary:
Update babel/register to latest version, fixing the bug that were preventing us from updating it previously.

Changelog: [Internal]

Differential Revision: D64245277

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 11, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64245277

vzaidman added a commit to vzaidman/react-native that referenced this pull request Oct 11, 2024
Summary:
X-link: facebook/metro#1371

Update `babel/register` to latest version, fixing the bug that were preventing us from updating it previously.

### The bug
In the pre-fix setup `babel/register` was installed twice:

Once in-
* **`xplat/js/node_modules`**
  * version `7.22.5` forced in `js/tools/metro/packages/metro-babel-register/package.json`.

And once in-
* **`xplat/js/react-native-github/node_modules`**
  * Version `7.24.6` as a dependency of `"metro-babel-register": "^0.81.0-alpha.2",`

When `js1 run` is launched, there's a call to
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/tools/metro/packages/metro/src/index.js?lines=19
and from there to
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/tools/metro/packages/metro-babel-register/src/babel-register.js?lines=164-171
which calls
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/tools/babel-register/index.js?lines=23-29
and registers the three paths:
* `xplat/js/react-native-github/packages`
* `xplat/js/RKJSModules`
* `xplat/js/tools/(?!node_modules/)`

by calling
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/tools/metro/packages/metro-babel-register/src/babel-register.js?lines=50
---
---
Later, when Metro is setting up `react-native/dev-middleware` in `fbsource`, it was calling-
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/react-native-github/packages/dev-middleware/src/index.js?lines=16-18
which was calling
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/react-native-github/scripts/build/babel-register.js?lines=28-36
which was setting up `babel/register` to transpile only one path instead of the three that were already set up:
`PACKAGES_DIR == `**`"xplat/js/react-native-github/packages"`**
by calling
https://www.internalfb.com/code/fbsource/[ba7f25a7faa2]/xplat/js/tools/metro/packages/metro-babel-register/src/babel-register.js?lines=50

---
---

Now the only reason it worked when `babel/register` was installed twice in `node_modules` of different packages was that this latter call to `babel/register` **was COMPLETELY IGNORED** and we kept using the real `babel/register` that was properly set-up with three paths before.

This means that when we previously tried to update `babel/register` and dedupe it, this call was not ignored. It was instead actually re-writing the paths that `babel/register` was transpiling to include only **`"xplat/js/react-native-github/packages"`** so we saw error coming out of the other two paths.

This means that if we update `xplat/js/react-native-github` to use the latest `metro-babel-register` the issue will come back. This is because both the version specified and the version in fbsource will have the same version of `babel/register` (the pinned `7.22.5`).

### The fix
1. Update and dedupe `babel/register`.
1. Make `js/react-native-github/scripts/build/babel-register.js` that is responsible for calling `babel/register` respect `process.env.FBSOURCE_ENV` and call the `xplat/js` monorepo instead of the `react-native-github` monorepo only.

Changelog: [Internal]

Differential Revision: D64245277
vzaidman added a commit to vzaidman/react-native that referenced this pull request Oct 11, 2024
Summary:

X-link: facebook/metro#1371

Update `babel/register` to latest version, fixing the bug that were preventing us from updating it previously.

Differential Revision: D64245277
Summary:
X-link: facebook/react-native#46987


Update `babel/register` to latest version, fixing the bug that were preventing us from updating it previously.

Differential Revision: D64245277
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64245277

vzaidman added a commit to vzaidman/react-native that referenced this pull request Oct 11, 2024
Summary:

X-link: facebook/metro#1371

Update `babel/register` to latest version, fixing the bug that were preventing us from updating it previously.

Differential Revision: D64245277
@vzaidman vzaidman changed the title import correct babel register fix how @babel/register is used and update it Oct 11, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 52bb874.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 14, 2024
Summary:
Pull Request resolved: #46987

X-link: facebook/metro#1371

Update `babel/register` to latest version, fixing the bug that were preventing us from updating it previously.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D64245277

fbshipit-source-id: f3d07b06a11fbe3a0ed28e22f5b687541782dda9
@vzaidman vzaidman deleted the export-D64245277 branch October 17, 2024 14:31
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. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants