Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move metro-minify-terser dependency to metro-transform-worker, fix pnpm (etc) resolution #1199

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

Summary:
Related: #1172

This is the minimally invasive change to fix resolution of the default minifierPath: 'metro-minify-terser', especially under isolated node_modules layouts.

minifierPath is required/resolved only from metro-transform-worker:

Per the current docs for minifierPath, a module specifier relative to metro-transform-worker is explicitly acceptable:

Type: string (default: 'metro-minify-terser')
Path, or package name resolvable from metro-transform-worker, to the minifier that minifies the code after transformation.

Unlike #1172 (thanks tido64 for flagging), this doesn't modify the defaults and can be released in a patch release. The approach in that PR (using fully resolved paths in config) may be the better long-term fix though, so this patch shouldn't be regarded as superseding it.

Changelog:

 - **[Fix]:** Move `metro-minify-terser` dependency to fix resolution under isolated node_modules (pnpm, etc).

Differential Revision: D53000650

…x pnpm (etc) resolution

Summary:
Related: facebook#1172

This is the minimally invasive change to fix resolution of the default `minifierPath: 'metro-minify-terser'`, especially under isolated node_modules layouts.

`minifierPath` is required/resolved only from `metro-transform-worker`:
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/utils/getMinifier.js#L22
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/index.js#L656

Per the [current docs for `minifierPath`](https://metrobundler.dev/docs/configuration/#minifierpath), a module specifier relative to `metro-transform-worker` is explicitly acceptable:

> Type: string (default: 'metro-minify-terser')
> Path, or package name resolvable from metro-transform-worker, to the minifier that minifies the code after transformation.

Unlike facebook#1172 (thanks tido64 for flagging), this doesn't modify the defaults and can be released in a patch release. The approach in that PR (using fully resolved paths in config) may be the better long-term fix though, so this patch shouldn't be regarded as superseding it.

Changelog:
```
 - **[Fix]:** Move `metro-minify-terser` dependency to fix resolution under isolated node_modules (pnpm, etc).
```

Differential Revision: D53000650
@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 Jan 24, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 8e26460.

@robhogan robhogan mentioned this pull request Jan 29, 2024
robhogan added a commit that referenced this pull request Jan 29, 2024
…x pnpm (etc) resolution (#1199)

Summary:
Pull Request resolved: #1199

Related: #1172

This is the minimally invasive change to fix resolution of the default `minifierPath: 'metro-minify-terser'`, especially under isolated node_modules layouts.

`minifierPath` is required/resolved only from `metro-transform-worker`:
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/utils/getMinifier.js#L22
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/index.js#L656

Per the [current docs for `minifierPath`](https://metrobundler.dev/docs/configuration/#minifierpath), a module specifier relative to `metro-transform-worker` is explicitly acceptable:

> Type: string (default: 'metro-minify-terser')
> Path, or package name resolvable from metro-transform-worker, to the minifier that minifies the code after transformation.

Unlike #1172 (thanks tido64 for flagging), this doesn't modify the defaults and can be released in a patch release. The approach in that PR (using fully resolved paths in config) may be the better long-term fix though, so this patch shouldn't be regarded as superseding it.

Changelog:
```
 - **[Fix]:** Move `metro-minify-terser` dependency to fix resolution under isolated node_modules (pnpm, etc).
```

Reviewed By: huntie

Differential Revision: D53000650

fbshipit-source-id: 251f52c17af58c88ebedb387ac92ecbe788772ea
robhogan added a commit that referenced this pull request Jan 30, 2024
…x pnpm (etc) resolution (#1199)

Summary:
Pull Request resolved: #1199

Related: #1172

This is the minimally invasive change to fix resolution of the default `minifierPath: 'metro-minify-terser'`, especially under isolated node_modules layouts.

`minifierPath` is required/resolved only from `metro-transform-worker`:
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/utils/getMinifier.js#L22
 - https://github.com/facebook/metro/blob/v0.80.4/packages/metro-transform-worker/src/index.js#L656

Per the [current docs for `minifierPath`](https://metrobundler.dev/docs/configuration/#minifierpath), a module specifier relative to `metro-transform-worker` is explicitly acceptable:

> Type: string (default: 'metro-minify-terser')
> Path, or package name resolvable from metro-transform-worker, to the minifier that minifies the code after transformation.

Unlike #1172 (thanks tido64 for flagging), this doesn't modify the defaults and can be released in a patch release. The approach in that PR (using fully resolved paths in config) may be the better long-term fix though, so this patch shouldn't be regarded as superseding it.

Changelog:
```
 - **[Fix]:** Move `metro-minify-terser` dependency to fix resolution under isolated node_modules (pnpm, etc).
```

Reviewed By: huntie

Differential Revision: D53000650

fbshipit-source-id: 251f52c17af58c88ebedb387ac92ecbe788772ea
# for free to join this conversation on GitHub. Already have an account? # 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