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.

Sign up for GitHub

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

Jump to bottom

update babel and fix tests accordingly #1343

Closed
wants to merge 2 commits into from
Closed

update babel and fix tests accordingly #1343

wants to merge 2 commits into from
+873 −219

Conversation

vzaidman
Copy link
Contributor

@vzaidman vzaidman commented Sep 2, 2024 •
edited
Loading

Updated babel packages in package.json, removed yarn.lock files and recreated them by running yarn install also fixed the following:

  • Updated jest snapshot tests failing
  • Updated babel types and corrected typings accordingly:
    • The latest babel 7 introduces the following features we need to adjust our types to:
      • JSXNamespacedName is removed from valid CallExpression args (PR)
        • JSXNamespacedName is used for namespaced XML properties in things like <div namespace:name="value">, but fn(namespace:name) doesn't make any sense.
      • Dynamic imports are enabled behind a new flag createImportExpressions (PR), introducing calls such as import(foo, options). These complicate the expected values passed to import to be more than just strings.
        • Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

Differential Revision: D61543660

Also merged #1340 - "re-implement babel traverse cache pollution bug workaround using a second copy of babel traverse"

@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 Sep 2, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@vzaidman @facebook-github-bot
update babel and fix tests accordingly (#1343)
583182c 
Summary:
Pull Request resolved: #1343

Updated babel packages in all `package.json` and fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.
### (D61388737) fixed js1 codeshift tool
Babel @ v7 changes
RestProperty and SpreadProperty
to be
RestElement and SpreadElement.
babel/babylon#384

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

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

vzaidman added a commit to facebook/react-native that referenced this pull request Sep 4, 2024
@vzaidman @facebook-github-bot
update babel and fix tests accordingly
8686153 
Summary:
X-link: facebook/metro#1343

Updated all **babel** packages in all `package.json` across the repo and ran `npx yarn-deduplicate yarn.lock`. Then fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

### Added a new metro ENV ignore
`BROWSERSLIST_ROOT_PATH` is set to `""` explicitly in `xplat/js/BUCK`
and then ignored in
`js/tools/metro-buck-transform-worker/src/EnvVarAllowList.js`

Differential Revision: D61543660
vzaidman added a commit to facebook/react-native that referenced this pull request Sep 4, 2024
@vzaidman @facebook-github-bot
update babel and fix tests accordingly
e34ae50 
Summary:
X-link: facebook/metro#1343

Updated all **babel** packages in all `package.json` across the repo and ran `npx yarn-deduplicate yarn.lock --scopes babel`. Afterwards, fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

### Added a new metro ENV ignore
`BROWSERSLIST_ROOT_PATH` is set to `""` explicitly in `xplat/js/BUCK`
and then ignored in
`js/tools/metro-buck-transform-worker/src/EnvVarAllowList.js`

Differential Revision: D61543660
vzaidman added a commit to facebook/react-native that referenced this pull request Sep 4, 2024
@vzaidman @facebook-github-bot
update babel and fix tests accordingly (#46295)
057ae34 
Summary:
Pull Request resolved: #46295

X-link: facebook/metro#1343

Updated all **babel** packages in all `package.json` across the repo and ran `npx yarn-deduplicate yarn.lock --scopes babel`. Afterwards, fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

### Added a new metro ENV ignore
`BROWSERSLIST_ROOT_PATH` is set to `""` explicitly in `xplat/js/BUCK`
and then ignored in
`js/tools/metro-buck-transform-worker/src/EnvVarAllowList.js`

Differential Revision: D61543660
vzaidman added a commit to facebook/react-native that referenced this pull request Sep 5, 2024
@vzaidman @facebook-github-bot
update babel and fix tests accordingly (#46295)
25c8c2f 
Summary:
Pull Request resolved: #46295

X-link: facebook/metro#1343

Updated all **babel** packages in all `package.json` across the repo and ran `npx yarn-deduplicate yarn.lock --scopes babel`. Afterwards, fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

### Added a new metro ENV ignore
`BROWSERSLIST_ROOT_PATH` is set to `""` explicitly in `xplat/js/BUCK`
and then ignored in
`js/tools/metro-buck-transform-worker/src/EnvVarAllowList.js`

Reviewed By: robhogan

Differential Revision: D61543660
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Sep 5, 2024
@vzaidman @facebook-github-bot
update babel and fix tests accordingly (#46295)
b371014 
Summary:
Pull Request resolved: #46295

X-link: facebook/metro#1343

Updated all **babel** packages in all `package.json` across the repo and ran `npx yarn-deduplicate yarn.lock --scopes babel`. Afterwards, fixed the following issues appearing as a result of that (squashed the following initially separate diffs to make this packages update work atomically):
### (D61336392) updated jest snapshot tests failing
### (D61336393) updated babel types and corrected typings accordingly
The latest babel 7 introduces the following features we need to adjust our types to:
* `JSXNamespacedName` is removed from valid `CallExpression` args ([PR](babel/babel#16421))
  * `JSXNamespacedName` is used for namespaced XML properties in things like `<div namespace:name="value">`, but `fn(namespace:name)` doesn't make any sense.
* Dynamic imports are enabled behind a new flag `createImportExpressions` ([PR](babel/babel#15682)), introducing calls such as `import(foo, options)`. These complicate the expected values passed to `import` to be more than just strings.
  * Since these are behind a flag that is not expected to be enabled, we can throw an error for now and whoever uses it can add a support to it if needed later.

### Added a new metro ENV ignore
`BROWSERSLIST_ROOT_PATH` is set to `""` explicitly in `xplat/js/BUCK`
and then ignored in
`js/tools/metro-buck-transform-worker/src/EnvVarAllowList.js`

Reviewed By: robhogan

Differential Revision: D61543660

fbshipit-source-id: abbcab72642cf6dc03eed5142eb78dbcc7f63a86
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 2c3a22a.

@huntie huntie mentioned this pull request Oct 1, 2024
Closed
huntie added a commit to huntie/metro that referenced this pull request Oct 1, 2024
@huntie @facebook-github-bot
fix: Pin @babel/register dependency to 7.22.5
af5ef19 
Summary:
This fixes the current state in fbsource, where the bump to this dependency breaks existing run-from-source behaviour when we bump Metro (and `metro-babel-register`) to latest (where facebook#1343 recently bumped all Babel deps).

No change to other bumped Babel packages — want to minimally unblock facebook/react-native#46703. As this is fbsource-only, no new release of Metro is needed.

Changelog: [Internal]

Reviewed By: vzaidman

Differential Revision: D63694664
huntie added a commit to huntie/metro that referenced this pull request Oct 1, 2024
@huntie @facebook-github-bot
Pin @babel/register dependency to 7.22.5 (facebook#1359)
b041c69 
Summary:
Pull Request resolved: facebook#1359

This fixes the current state in fbsource, where the bump to this dependency breaks existing run-from-source behaviour when we bump Metro (and `metro-babel-register`) to latest (where facebook#1343 recently bumped all Babel deps).

No change to other bumped Babel packages — want to minimally unblock facebook/react-native#46703. As this is fbsource-only, no new release of Metro is needed.

Changelog: [Internal]

Reviewed By: vzaidman

Differential Revision: D63694664
@vzaidman vzaidman deleted the export-D61543660 branch October 1, 2024 14:48
facebook-github-bot pushed a commit that referenced this pull request Oct 1, 2024
@huntie @facebook-github-bot
Pin @babel/register dependency to 7.22.5 (#1359)
03724c0 
Summary:
Pull Request resolved: #1359

This fixes the current state in fbsource, where the bump to this dependency breaks existing run-from-source behaviour when we bump Metro (and `metro-babel-register`) to latest (where #1343 recently bumped all Babel deps).

No change to other bumped Babel packages — want to minimally unblock facebook/react-native#46703. As this is fbsource-only, no new release of Metro is needed.

Changelog: [Internal]

Reviewed By: vzaidman

Differential Revision: D63694664

fbshipit-source-id: 83af0807570c2d2b6517d4bd4a668550463190b0
jerone
jerone reviewed Nov 28, 2024
View reviewed changes
packages/metro-source-map/package.json
"@babel/traverse": "^7.20.0",
"@babel/types": "^7.20.0",
"@babel/traverse": "^7.25.3",
"@babel/traverse--for-generate-function-map": "npm:@babel/traverse@^7.25.3",
Copy link

@jerone jerone Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@vzaidman Why was this dependency with referrer created?

Copy link

@jerone jerone Nov 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Our security tools are highlighting this as a potential security issue.

detected resolved URL for package with a different name: @babel/traverse--for-generate-function-map
    expected: @babel/traverse--for-generate-function-map
    actual: @babel/traverse

Copy link
Contributor Author

@vzaidman vzaidman Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@jerone - added the info about that to the PR summary. It was introduced in #1340 - "re-implement babel traverse cache pollution bug workaround using a second copy of babel traverse".

The tool flagging it as a potential security issue is concerned about "detected resolved URL for package with a different name" but this is totally expected, so I think you can ignore or mark this concern as resolved.

Copy link

Choose a reason for hiding this comment

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

Thanks for the update. I have whitelisted this change.

The tool that giving the warning was lockfile linting: https://github.com/lirantal/lockfile-lint/tree/main/packages/lockfile-lint-api#validators

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerone jerone jerone left review comments

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vzaidman @facebook-github-bot @jerone