Repository navigation
Conversation
Summary: This was added to `loadConfig.js` but is missing in other places. While it's arguably not useful in many cases in a "standard" Metro setup with Metro's own packages, it'd be nice to have this behave consistently even with transpiled ESM output (or with require-ESM in Node). Specifically this affects: - metro-file-map plugin workers that are loaded with `require` - metro-file-map's main worker loading - metro's asset plugins - metro-transform-worker's `getMinifier` implementation - metro-transform-worker's `babelTransformerPath` loading This is probably specifically interesting for `babelTransformerPath` which is often overridden by third-party setups. Changelog: [Fix] Unwrap ES modules when requiring metro-file-map workers, asset plugins, the minifier, or the babel transformer Pull Request resolved: #1675 Test Plan: - n/a: This will likely be best left untested, but is a consistent implementation that matches ESM->CJS semantics, also used in `loadConfig.js` Reviewed By: javache Differential Revision: D102510513 Pulled By: robhogan fbshipit-source-id: 863b53901d90e06853f96fa04b2b129c85b316e8 (cherry picked from commit 0e9d7aa)
…1712) Summary: Pull Request resolved: #1712 babel/plugin-transform-modules-systemjs versions >= 7.12.0 and <= 7.29.3 are affected by CVE-2026-44728 (GHSA-fv7c-fp4j-7gwp), a HIGH severity vulnerability. The metro website resolves this package at 7.25.9 via babel/preset-env from Docusaurus. This bumps the package to ^7.29.4, the first patched version. Reviewed By: robhogan Differential Revision: D104687111 fbshipit-source-id: 19ec44324488a79fb078c2939f8c05573c0c8695 (cherry picked from commit 25bad99)
Summary: Related PR for `expo/expo` fork: expo/expo#45648 Supersedes #1696 Cross-device path normals are intended to be relative paths from the rootDir. If the rootDir is at `C:\project` then a cross-device normal path at `D:\temp` is represented as `..\..\D:\temp`. This assumption is broken in several places, like `normalToAbsolute` printing, leading to `C:\D:\` paths that are invalid. Changelog: [Fix] Fix Windows path handling for cross-drive paths Pull Request resolved: #1711 Test Plan: - Unit tests were adjusted to demonstrate all failing cases - **Note:** Invalid unit tests have been removed for paths that rise above the filesystem root (they're assumed to be impossible, and their output doesn't have to be deterministic/correct) Reviewed By: robhogan Differential Revision: D104700077 Pulled By: CalixTang fbshipit-source-id: e77baa4e66495b50ef41b260cec60bcc832d4a6a (cherry picked from commit 6e6b52c)
…n rather than calls to redirectModulePath Summary: Currently, `metro-resolver` tests assert on and mock calls to `redirectModulePath`, which since [5e96d17](5e96d17) (v0.81.0) is a fixed implementation detail, rather than a user-configurable context method. Refactor tests to assert on observable behaviour by creating mock `package.json`s, which makes these tests more robust to upcoming internal optimisations. Changelog: [Internal] ___ Differential Revision: D102514698 fbshipit-source-id: 05c47b37ea07d2d42128b20c4d15148a8f35a634 (cherry picked from commit 922ee8b)
…ft closest package scope to root resolver for reuse
Summary:
On the bare-specifier hot path, replace the top-level `redirectModulePath(context, specifier)` wrapper call with a direct `getPackageForModule(originModulePath)` + `matchSubpathFromMainFields(...)`.
Perf: `resolve()` has already excluded relative/absolute specifiers by this point, so the `redirectModulePath` wrapper was re-deriving facts the caller already knew — an extra call frame, a redundant `path.isAbsolute()` and `.startsWith('.')`, and an unconditional `isRelativeImport()/path.isAbsolute()` on the result that now only runs on the rare actual-redirect case. The two genuinely costly operations (the closest-package lookup and the main-fields match) are unchanged, so this is a small saving — ~1.5% resolver-only CPU (see test plan).
More importantly, this surfaces the closest `package.json` to the origin (`closestPackageToOrigin`) in the root resolver, where it can be reused by follow-up diffs rather than being re-derived inside `redirectModulePath`.
In particular, we'll reuse closest package scope to implement `PACKAGE_SELF_RESOLVE` at negligible cost.
Changelog: Internal
Reviewed By: GijsWeterings, huntie
Differential Revision: D102515190
fbshipit-source-id: acc962617e38e1c56b85196ed71a5b010abe2c17
(cherry picked from commit 7529585)
…s, improve overall resolver perf ~5%
Summary:
`matchSubpathFromMainFields` is called on every resolution attempt (per source-extension via `redirectModulePath`, and per package entry point), and on every call it allocated a `.map()` array, a `.filter()` array, and an `Object.assign({}, ...spread)` - even in the overwhelmingly common case where the package declares no object-valued main field (no "browser" map) and the function returns `null`.
Instead, rebuild the merged replacement map with a single reverse loop over `mainFields` (preserving the prior "earlier mainFields win on key conflict" semantics) such that nothing is allocated (`null` is still returned) unless and until an object-valued field is actually found. The list of subpath variants is likewise only built in that rare matched case.
This improves the perf of `matchSubpathFromMainFields` by ~20% and ~5% on resolutions overall, based on Meta's product graph.
```
- **[Performance]**: Refactor to reduce allocs for a ~5% faster resolver
Reviewed By: huntie
Differential Revision: D107024445
fbshipit-source-id: cec2261dc588590eb12bb413a7bbb6fc6869401c
(cherry picked from commit 9341463)
…ecifier -> absolute path
Summary:
While doing some refactoring here I noticed some nonsensical handling of the case where a browser-spec `package.json` tries to redirect a bare specifier to an absolute path, eg:
```
// /project/package.json
{
"browser": {
"foo-pkg": "/var/foo"
}
}
```
This currently causes the resolver to redirect to `/project/var/foo` - *joining* the absolute path onto the end of the origin-relative path.
This is surely never the desired behaviour, and is outside the [`browser` field spec](https://github.com/defunctzombie/package-browser-field-spec#replace-specific-files---advanced).
Rather than "fix" this and support something new in an increasingly outdated spec (`browser` redirects have been effectively replaced by `exports`/`imports` maps), here we just replace what was broken with a graceful, actionable rejection.
Although this changes observable resolver behaviour, I think it's safe to treat this as non-breaking because the previous behaviour was so unexpected.
```
- **[Fix]**: Resolver: Gracefully reject bare->absolute redirects rather than failing on a spurious path
```
Reviewed By: huntie
Differential Revision: D102568988
fbshipit-source-id: 5bf8d6c3f5a26e0b3ea2fd3f49a3dd9fa4113663
(cherry picked from commit e1f4d61)
… nested/first-party package.jsons (#1720) Summary: Pull Request resolved: #1720 The [browser spec](https://github.com/defunctzombie/package-browser-field-spec) redirect for bare specifiers (`"browser": {"foo-pkg": "./shims/foo.js"}`) was previously rebased against a path derived by slicing `originModulePath` after the last `node_modules/` segment. That happens to be correct only in the (common) case that the closest `package.json` is the one at the root of a containing `node_modules` package. However: - It's legal for an NPM package to contain nested `package.json` files and those may contain redirects - in which case their targets are relative to themselves, not to the package root `package.json`. - For origin modules that aren't located inside a `node_modules` tree (e.g. project-local source), `lastIndexOf('node_modules/')` returns `-1`, producing a malformed root path and either failed resolutions or surprising successes. This diff replaces that derivation with `closestPackageToOrigin.rootPath`, which we already look up at the top of `resolve`. Behaviour is identical in the already-working cases, and corrected otherwise. Matches webpack's `AliasFieldPlugin`, which rebases against `request.descriptionFileRoot`. Changelog: ``` - **[Fix]**: Fix browser-spec redirects of bare specifier to relative path, where `package.json` is not directly under `node_modules/<pkg>/` ```` Reviewed By: huntie Differential Revision: D105485945 fbshipit-source-id: 0b0b0b985294024d64305a15720ecc9007658d16 (cherry picked from commit 385a946)
…#1721) Summary: Pull Request resolved: #1721 Add support to Metro's resolver for [self-referencing](https://nodejs.org/api/packages.html#self-referencing-a-package-using-its-name). A package can import itself by name, resolving via its own `exports` field. Per the [Node ESM resolution algorithm](https://nodejs.org/api/esm.html#resolution-algorithm-specification) (6.5 `PACKAGE_SELF_RESOLVE`), if the origin module lives in a package whose `name` matches the bare specifier and which declares `exports`, the request is routed through `PACKAGE_EXPORTS_RESOLVE` against that package, in preference to hierarchical `node_modules` lookup. (In particular, this allows `react-native`'s own source to `import {Platform} from 'react-native'`, which *may* be the approach we take to fix broken platform inlining) Gated behind the existing `unstable_enablePackageExports` flag (enabled by default). When on, the resolver attempts self-resolve before hierarchical lookup; on `PackagePathNotExportedError` it follows Metro's existing nonstrict policy of warning and falling through to legacy resolution, consistent with how non-self bare specifiers behave today. This nonstrict fallback will be removed in future. Matches webpack's `SelfReferencePlugin` and Node's own behaviour. ``` - **[Feature]**: Resolver: support self-referencing packages ([`PACKAGE_SELF_RESOLVE`](https://nodejs.org/api/esm.html#resolution-algorithm-specification)) ``` Reviewed By: huntie, motiz88 Differential Revision: D105485943 fbshipit-source-id: 257090425208ef418d9b8c9cc3968d9d1df839cf (cherry picked from commit 06f1de1)
Summary:
`HttpStore.#withRetries(fn, endpoint)` is called for both endpoints — `get()` passes `#getEndpoint`, `set()` passes `#setEndpoint` — and it correctly consults the passed `endpoint` for the `maxAttempts === 1` short-circuit. However, the `backOff` options hardcoded `this.#getEndpoint` for `numOfAttempts`, `retryStatuses`, and `retryNetworkErrors`.
As a result, when an `HttpStore` is constructed with the documented split `{ getOptions, setOptions }` form, all `set()`/PUT retries silently use the **read** endpoint's retry configuration — `setOptions.maxAttempts`, `setOptions.retryStatuses`, and `setOptions.retryNetworkErrors` are ignored. There's also an internal inconsistency: the `maxAttempts === 1` early-return reads the (set) endpoint, but `numOfAttempts` then read `#getEndpoint.maxAttempts`.
The fix uses the passed-in `endpoint` consistently.
Pull Request resolved: #1751
Test Plan:
The existing retry tests only use the single-options form via `get()`, where `#getEndpoint === #setEndpoint`, so they can't catch this. Added a regression test that builds the store with the split form (reads: `maxAttempts: 1`; writes: `maxAttempts: 2, retryStatuses: [503]`) and asserts `set()` retries a 503:
- Before: `set()` uses the read config (no retry) and throws the 503 → test fails.
- After: `set()` retries per `setOptions` and resolves → test passes.
```
yarn jest packages/metro-cache/src/stores/__tests__/HttpStore-test.js
# Tests: 19 passed (full metro-cache package: 45 passed)
```
Reviewed By: Abbondanzo
Differential Revision: D109932333
Pulled By: robhogan
fbshipit-source-id: d7cdf9f1f128a611424df001ff885093682ed1ab
(cherry picked from commit 22fa6dd)
…1754) Summary: When `mergeConfig(base, ...configs)` encounters a config that resolves to a `Promise` (e.g. an async `metro.config.js` / a config function returning a promise) and at least one more config follows it, it hands off to `mergeConfigAsync` with two defects: 1. **Missing spread.** `mergeConfigAsync(baseConfig, ...reversedConfigs)` takes a rest parameter, but the call passed `reversedConfigs.toReversed()` as a single array argument. The remaining configs end up wrapped one level too deep, so they're merged as a bogus object (producing junk keys like `"0"`) instead of being applied as configs. 2. **Dropped accumulator.** `currentConfig` (the base merged with every config to the *left* of the async one) was discarded — the unresolved promise was passed as the base, so all prior merges were lost. The fix spreads the remaining configs into the rest parameter and folds `currentConfig` into the resolved async config so both the accumulated state and the trailing overrides are preserved. Pull Request resolved: #1754 Test Plan: The existing `mergeConfig` tests only cover sync configs, so this path was uncovered. Added a regression test (an async config function followed by a trailing override) asserting the base, the async config, and the trailing override all survive: ``` yarn jest packages/metro-config/src/__tests__/mergeConfig-test.js # Tests: 19 passed (full metro-config package: 40 passed) ``` - Before: `result.server.port` is `undefined` (base dropped); trailing override mangled. - After: all three configs merge correctly. Reviewed By: huntie, robhogan Differential Revision: D110207664 Pulled By: rozele fbshipit-source-id: 804041a89c05a08aee0823e91cf22f4539e927ca (cherry picked from commit 9dbe0cc)
meta-cla
Bot
added
the
CLA Signed
Summary: Pull Request resolved: #1769 The build script previously only excluded `__tests__`, and had an incorrect comment that `IGNORE_PATTERNS` was for passthrough, whereas it does actually exclude files from the build. This expands to an array of patterns and covers other non-prod files. Changelog: Internal Reviewed By: huntie Differential Revision: D111232823 fbshipit-source-id: 0e994803d2f5b036e4208686d28775a780ea442f Adapted for 0.84.x: Uses micromatch, as 0.84.x's build script predates the switch to path.matchesGlob. (cherry picked from commit bbdf032)
Summary: Pull Request resolved: #1770 X-link: react/react-native#57488 The generated Flow libdef for `babel/types` declared `duplicates` as a required parameter on `getBindingIdentifiers` and `getOuterBindingIdentifiers`. It is actually optional, both at runtime (only read as a truthy flag, defaulting to the non-duplicate map) and in the upstream TS types: https://github.com/babel/babel/blob/7.x/packages/babel-types/src/retrievers/getBindingIdentifiers.ts#L17-L36 This prevented legitimate single-argument use later in this stack. Separating it for export to RN + Metro. Changelog: [Internal] Reviewed By: GijsWeterings Differential Revision: D111056985 fbshipit-source-id: f3f07617e2fb010b08e83d7d012800a884f5bd15 (cherry picked from commit 25e322a)
Summary: Pull Request resolved: #1768 Add static import/export plugin (`experimentalImportSupport`) support for namespace re-exports, i.e. `export * as Name from "module"`. This was added to Expo's fork in expo/expo#38055, (h/t krystofwoldrich). Despite this being perfectly valid syntax it currently fails to transform under `experimantalImportSupport`, with: ``` Cannot read properties of undefined (reading 'name') at <..>/metro-transform-plugins/src/import-export-plugin.js:289:67 ``` (On access of `local.name`, because this type of export has no `local` binding) ## Spec ECMA-262 includes namespace re-export syntax in the ExportDeclaration grammar: https://tc39.es/ecma262/#prod-ExportDeclaration ## Implementation Metro's existing `experimentalImportSupport` transform already has namespace import helper plumbing via `importAll`. This change reuses that helper and then emits an explicit export assignment for the namespace object, keeping the change isolated to the static transform path. ``` - **[Experimental]** Support `export * as Name from "module"` under `experimentalImportSupport` ``` Reviewed By: huntie Differential Revision: D111013905 fbshipit-source-id: 101c89427b04263e641c769f4ed4c6641a18af09 (cherry picked from commit 59a1072)
…est spread (#1772) Summary: Pull Request resolved: #1772 Fixes #1460 Fixes two related bugs in Metro's `experimentalImportSupport`. ## Exports with inline relabelling ```js export const { A: B } = {}; ``` **Currently transforms to** ```js const { A: B } = {}; exports.A = A; ``` Which is invalid: `A` is a reference error. **After this diff** ```js const { A: B } = {}; exports.A = B; ``` ## Exports with spreads Currently both of these fail to transform: **Object spread** ```js export const {A, ...others} = {A: 'A', B: 'B', C: 'C'}; ``` Plugin throws `TypeError: Cannot read properties of undefined (reading 'name')` when attempting to iterate over the object for key: val pairs. **Array spread** ```js export const [A, ...others] = ['A', 'B', 'C']; ``` Plugin throws `Property name expected type of string but got undefined` after extracting an `undefined` name and then attempting to use it at as prop of `exports`. **After this diff** ```js const {A, ...others} = {A: 'A', B: 'B', C: 'C'}; exports.A = A; exports.others = others; ``` and ```js const [A, ...others] = ['A', 'B', 'C']; exports.A = A; exports.others = others; ``` ## Spec Almost goes without saying in this case - exported variable declarations should expose the declaration's bound names. ECMA-262 defines those names through Static Semantics: `BoundNames`, including `BindingPattern` forms: https://tc39.es/ecma262/#sec-static-semantics-boundnames ## Implementation Metro had naive `ObjectPattern`/`ArrayPattern` extraction that read object keys or array elements directly. This fixes the same issues Expo did in [#38058](expo/expo#38058) and [#38118](expo/expo#38118), but uses Babel's own `getBindingIdentifiers` for a simpler implementation. ## Changelog ``` - **[Experimental]**: experimentalImportSupport: Fix exports renamed by destructuring, and support exports of rest spreads. ``` Reviewed By: GijsWeterings, huntie Differential Revision: D111013897 fbshipit-source-id: 6f0b0e135f5014006d2d1b0851ae26903e21355b (cherry picked from commit 0dc5030)
…it exports, fix precedence of `export default` > `export * from` (#1773) Summary: Pull Request resolved: #1773 Fixes a bug in `experimentalImportSupport` where `export * from 'foo'` would incorrectly overwrite an `export default`, if `'foo'` also has a default export. # Example ## Input source ```js // foo.js (the star source) export default 'star default'; export const overridden = 'star named'; export const sourceOnly = 'source only'; ``` ```js // entry.js export * from 'foo'; export const overridden = 'explicit named'; export default 'explicit default'; ``` Per the ES spec, importing from `entry.js` should give `overridden === 'explicit named'`, `default === 'explicit default'`, and `sourceOnly === 'source only'`. Explicit exports take precedence over names supplied via `export *`, regardless of the position of export declarations. ## Before ```js 'use strict'; Object.defineProperty(exports, '__esModule', {value: true}); var _overridden = 'explicit named'; var _default = 'explicit default'; exports.overridden = _overridden; // 'explicit named' exports.default = _default; // 'explicit default' var _foo = require('foo'); for (var _key in _foo) { // star copy runs LAST... exports[_key] = _foo[_key]; // ...clobbers the explicit exports } ``` Runtime result (wrong): ```js exports.overridden // 'star named' ❌ should be 'explicit named' exports.default // 'star default' ❌ should be 'explicit default' exports.sourceOnly // 'source only' ✅ ``` ## After ```js 'use strict'; Object.defineProperty(exports, '__esModule', {value: true}); var _overridden = 'explicit named'; var _default = 'explicit default'; var _foo = require('foo'); for (var _key in _foo) { // star copy runs FIRST exports[_key] = _foo[_key]; } exports.overridden = _overridden; // explicit exports now win exports.default = _default; ``` Runtime result (spec-correct): ```js exports.overridden // 'explicit named' ✅ exports.default // 'explicit default' ✅ exports.sourceOnly // 'source only' ✅ ``` # Spec [ECMA-262 Source Text Module Record](https://tc39.es/ecma262/#sec-source-text-module-records-resolveexport-exportname-resolve-set) `ResolveExport` checks local `ExportEntries` and indirect `ExportEntries` before considering `StarExportEntries`, so explicit exports take precedence over names supplied through `export *`. # Provenance This is part of the change made in Expo's fork in [#38111](expo/expo#38111) (though that's mainly concerned with introducing live bindings), so this is "in the wild" already for Expo users. # Implementation This is just a straightforward ordering swap of the emitted assignments. # Changelog ``` - **[Experimental]**: `experimentalImportSupport` - fix precedence of `export default` over `export * from`. ``` Reviewed By: huntie Differential Revision: D111013892 fbshipit-source-id: 937f7aabdbe6760425072153f2093fe4e45a37e3 (cherry picked from commit e6b154f)
Summary: Pull Request resolved: #1774 In ESM, static imports (including `export * from`) must be evaluated before the current module's body. Metro's `import-export-plugin` currently evaluates named re-exports at their declared position within the body (by turning them into a CJS `require` at that posiiton), and `export *` after the body - both are incorrect. Move static re-export helper requires and export-star copies into the import prelude instead of leaving them at the original export declaration location or after the module body. ## `inlineRequires` Under `inlineRequires`, which runs after this plugin, evaluation of imported modules may be deferred back to where they were before this diff - contrary to the spec but by design for startup performance. ## Spec - Source Text Module Records collect imported and re-exported module specifiers as requested modules: https://tc39.es/ecma262/#sec-source-text-module-records - ECMA-262 `InnerModuleEvaluation` evaluates requested modules before executing the current module body: https://tc39.es/ecma262/#sec-innermoduleevaluation ## Expo Along with the previous diff, this is another change introduced in Expo last year in [#38111](expo/expo#38111), on by default for Expo apps. ## Changelog ```javascript - **[Experimental]**: `experimentalImportSupport`: Move `export from` imports to be evaluated at module prelude, rather than within/after module body ``` Reviewed By: huntie Differential Revision: D111013896 fbshipit-source-id: 069438ab66a206471fc9f948efc924ce881c8d6d (cherry picked from commit 27a5adc)
…ng the default export (#1777) Summary: Pull request resolved: #1777 Fix a spec conformance bug in `export * from 'foo'` under `experimentalImportSupport` (`import-export-plugin`). Currently, Metro re-exports all exports including the source module's default export - the default export should be excluded. ## Spec https://tc39.es/ecma262/multipage/ecmascript-language-scripts-and-modules.html#sec-getexportednames `8.d.i` ``` 8. For each ExportEntry Record exportEntry of module.[[StarExportEntries]], do a. Assert: exportEntry.[[ModuleRequest]] is not null. b. Let requestedModule be GetImportedModule(module, exportEntry.[[ModuleRequest]]). c. Let starNames be requestedModule.GetExportedNames(exportStarSet). d. For each element name of starNames, do -> i. If name is not "default", then 1. If exportedNames does not contain name, then a. Append name to exportedNames. ``` Node.js, Babel's `transform-modules-commonjs`, and browsers all conform to this spec. ## Fix Trivially `continue` through a key named "default" in our `exportAllTemplate`. NOTE: `export * as ns from 'foo'` is unaffected: it goes through the `ExportNamespaceSpecifier` / `importAll` path, and per spec the namespace object still exposes the source's default. ## Possible alternatives + benchmarks Compiled release HBC under SH trunk (`hermesc -O -emit-binary`) and run in the `hermes` VM. - One "op" = one full re-export of a module with 20 named exports + `default` (where noted). - Throughput = median of 9 runs × 250k ops. - Bytecode = marginal HBC size of the isolated `f(exports, REQUIRED)` body vs an empty-body baseline. | Form | Input | Throughput (ops/s) | stddev | rel | HBC bytes (Δ) | |---|---|--:|--:|--:|--:| | `for-in`, no `default` guard (re-exports default) | 20 + `default` | 99,602 | 1,041 | 0.95× | +96 | | `for-in` + `if (KEY === "default") continue;` (current) | 20 + `default` | 104,778 | 1,025 | 1.00× | +126 | | `for-in` + `if (KEY !== "default") { ... }` | 20 + `default` | 104,515 | 879 | 1.00× | +127 | | object spread + `delete default` | 20 + `default` | 91,709 | 583 | 0.88× | +148 | | `Object.keys` + indexed for-loop | 20 + `default` | 90,351 | 672 | 0.86× | +206 | | `Object.getOwnPropertyNames` + for-loop | 20 + `default` | 89,831 | 680 | 0.86× | +224 | | `for-in` + `hasOwnProperty` guard | 20 + `default` | 78,939 | 862 | 0.75× | +239 | | `Object.keys().forEach` | 20 + `default` | 73,746 | 346 | 0.70× | +273 | | `Object.assign` + `delete default` | 20 + `default` | 61,395 | 567 | 0.59× | +139 | | `for-in` + `default` guard (guard is pure overhead) | 20, **no** `default` | 104,080 | 538 | 0.99× | +126 | | `for-in`, no guard | 20, **no** `default` | 104,866 | 634 | 1.00× | +96 | **Conclusion:** both `for-in` guard styles are fastest and smallest of all semantically-equivalent forms (differing by ~3% throughput / 1 byte — within noise). - The `default` guard's runtime effect flips with the input: - **default present:** guard is **+5.2%** faster (104,778 vs 99,602) — skipping the `default` property write outweighs the per-key comparisons. - **default absent:** guard is **−0.7%** slower (104,080 vs 104,866) — pure overhead, nothing to skip. - The guard is order of magnitudes cheaper than prop assignment, adding it has negligible effect. - Adding the guard adds 5 bytes of HBC per occurrence (plus one-time 16 bytes for the "default" string table entry - which is there in a real app anyway). ## Changelog ``` - **[Experimental]** experimentalImportSupport: Fix `export * from` incorrectly re-exporting default exports ``` Reviewed By: huntie Differential Revision: D111910097 fbshipit-source-id: 5c93c35fa8fd17470bed14caae9f95347fccf66d (cherry picked from commit 11ad083)
Summary: I need to double-check this every time, so use some clearer labelling in opaque types + names + comments, where it's API neutral. Changelog: Internal Reviewed By: huntie Differential Revision: D114616012 fbshipit-source-id: a615e4467ba249f5a815d527457cd88407884bd9 (cherry picked from commit 42a6502)
Summary: Fast Refresh's dependency walk (`metroHotUpdateModule` in the require polyfill) assumes every module reachable through `inverseDependencies` is already present in the client module registry as an (uninitialised) `ModuleDefinition`. This is true for single-segment "plain" bundles, but doesn't hold for lazily defined segments. Currently these are only used in non-`dev` builds so Fast Refresh doesn't need to care about them, but in the spirit of bringing dev and prod as close together as possible, and unforking serialiser implementations in the process, we want to expose segmented bundles in dev. Fortunately it's quite easy - the pieces we need (`definingSegmentByModuleID`, `moduleDefinersBySegmentID`) are already in the runtime. We only need to define (note, not initialise/evalaute - we're not running user code) them, just in time. This extra step - `ensureModuleRegistered` - is entirely contained within the Fast Refresh pass so there's no prod impact at all, and it's a no-op for plain (eager) bundles. Changelog: ``` - **[Fix]** Fix Fast Refresh hitting undefined modules when using lazy (segmented/switch) bundles in dev ``` Reviewed By: vzaidman Differential Revision: D113665912 fbshipit-source-id: c648c0457d33b468ae73402b1a5c85b333bd2eaa (cherry picked from commit c70d0ae)
…fault Summary: `ci-info` was a direct runtime dependency of `metro` with exactly one use: choosing the default value of `watch` in `createFileMap`. Replace it with a local `isCIEnv()` helper reading `process.env.CI`, and drop the dependency along with its now-unused Flow libdef. This also fixes an opt-out bug. Metro pinned `ci-info@^2.0.0`, whose `isCI` is a plain truthiness check: ``` exports.isCI = !!(env.CI || env.CONTINUOUS_INTEGRATION || env.BUILD_NUMBER || env.RUN_ID || exports.name || false) ``` so `CI=false` and `CI=0` were both treated as "in CI" — the opposite of what someone setting them intends. `ci-info@3` later added an `env.CI !== 'false'` bypass for precisely this reason. `isCIEnv()` adopts the v3 semantics (and additionally honours `CI=0`, another conventional falsey spelling) without taking a major upgrade of a dependency we no longer want. The tradeoff is narrower CI detection: we no longer infer CI from `CONTINUOUS_INTEGRATION`, `BUILD_NUMBER`, `RUN_ID`, or ci-info's `vendors.json` list, so a CI that does not export `CI` (Jenkins, TeamCity) now defaults to `watch: true`. That is acceptable here because: - every mainstream CI (GitHub Actions, CircleCI, GitLab CI, Buildkite, Travis) sets `CI=true` — that is the convention this variable exists for; - this is only a *default*. `watch` is an explicit option on `runServer` / `DependencyGraph` / `createFileMap`, and `runBuild` already passes `watch: false`; - the cost of a miss is a filesystem watcher we did not need in a `metro serve` process, not incorrect output. `ci-info@3` remains in the lockfiles as a transitive dev dependency of Jest. Reviewed By: vzaidman Differential Revision: D116225836 fbshipit-source-id: f8d1a7a6cfc98face0ad99255ad63556a7e27f2e (cherry picked from commit 61e4592)
…lt on via `@react-native/metro-babel-transformer`) Summary: X-link: react/react-native#57973 `react-native/babel-preset` inlines `Platform.OS` and `Platform.select(...)` whenever it is handed a `platform` (since yesterday - react/react-native#57848 ), but this is over-zealous - `platform` is a pre-existing transform option whose intent is to set the transform target. In some cases (eg, to allow `Platform` mocking from tests), we don't want `Platform` inlining, even though `platform` may already be passed for other reasons. Gate the inlining behind a separate `inlinePlatform` opt-in, resolved as `options.inlinePlatform ?? babel.caller(...) ?? false`, mirroring how `platform` and `unstable_transformProfile` are already resolved. Metro already models this as a distinct transform option, so both Babel transformers pass it straight through. The caller channel covers the case where the preset is named in a `babel.config.js` and so receives no preset options at all. Bundling is unaffected: Metro sets `inlinePlatform` on every transform, so `Platform` continues to be inlined through `react-native/metro-babel-transformer`. Changelog: [General][Changed] - `Platform.OS` and `Platform.select(...)` inlining in `react-native/babel-preset` now requires the `inlinePlatform` option in addition to `platform` Reviewed By: GijsWeterings Differential Revision: D116281656 fbshipit-source-id: 0230cd74f38661862396391da62cf81b34773b85 (cherry picked from commit 6bbe095)
Summary: WARNING: Generated by Autopilot (alpha) — review carefully, verify the underlying claim before accepting. Agent: React Native Oncall Tests Fixer | Trajectory: https://www.internalfb.com/intern/devai/devmate/inspector/7753ac91-3b3f-43d3-a8aa-d9e336e50d0d/ | SC job: https://www.internalfb.com/intern/sandcastle/instance/67553994997036426/ --- The `react_native` Metro remote-cache warmer (`//tools/skycastle/workflows2/react_native/fb-metro-cache-warmer.sky:main`, test issue 237151770) has been failing at a 25-46% rate. The failing shard's log shows the actual cause once, at the top, followed by a long tail of consequences: ``` Uncaught exception: Error: read ECONNRESET at TLSWrap.onStreamRead (node:internal/stream_base_commons:216:20) Reporting error to the React Native team. ... 38 of 112 failures exceeds tolerance of 10%. Failing job. FetchError: request to http://localhost:37907/fb-metro-cache-stats failed, reason: connect ECONNREFUSED ::1:37907 ``` `HttpStore.#setOnce` attaches `req.on('timeout')` but — unlike `#getOnce` — never attaches `req.on('error')`. A Node `ClientRequest` that emits `'error'` with no listener throws it as an uncaught exception. Because the event is emitted asynchronously, it escapes the `new Promise` executor entirely, so `reject` is never called: `Cache.set`'s try/catch cannot see it and it lands in the CLI's `uncaughtException` handler, which calls `process.exit(1)`. That kills the Metro server the warmer spawned. Every bundle still queued then fails with `connect ECONNREFUSED` against the now-dead local server, which is why a single socket reset produces dozens of bundle failures and trips the 10% tolerance. The `ECONNREFUSED` tail is the symptom, not the cause — it accounts for 114 of the 177 recent failed runs. The reset itself is expected and benign: writes go to Manifold over TLS through a `keepAlive` agent with `maxSockets: 64`, so a pooled socket being reaped server-side races the next PUT. `setOptions` already specifies `retryNetworkErrors: true, maxAttempts: 3` for exactly this, but that retry path was dead for socket errors — a failure that never rejects can never be retried. Attach `req.on('error')` in `#setOnce`, mirroring `#getOnce`, rejecting with a `NetworkError` so `#withRetries` can retry it. Also attach `gzip.on('error')`: `gzip.pipe(req)` does not forward errors, so a destroyed request mid-upload can leave the upload stream emitting unhandled `'error'` too. This also restores the warmer's documented intent — `fill-http-cache` already treats `cache_write_error` as non-fatal ("Cache warming is best-effort") — which the uncaught path was defeating, since the error never became a `cache_write_error` event. Reviewed By: javache Differential Revision: D116631955 fbshipit-source-id: b80baceb2b812d5f0b885175299233c73a5cd637 Adapted for 0.84.x: Tests mock `http`, not `node:http`. (cherry picked from commit fad3b88)
Summary: Metro replaces `Platform.OS` with a string literal unless it is the direct left side of an assignment. Update/delete expressions, destructuring assignment targets, and direct or nested `for...in` / `for...of` targets can therefore be changed semantically or make Babel abort with an invalid literal assignment target. Walk the member expression's ancestors and preserve it whenever the containing expression is the left side of an assignment or loop, or the argument of an update/delete operation. The React Native Babel preset has a parallel transform; companion [React Native PR #58252](react/react-native#58252) applies the same guard there. Changelog: ``` - **[Fix]**: Preserve `Platform.OS` write targets during constant inlining ``` Pull Request resolved: #1890 Test Plan: - Added one combined regression for update/delete, array/object destructuring assignments, and direct/nested `for...in` and `for...of` targets. - Exact baseline aborts on the direct `for...of` target; the fixed suite preserves all eight forms. - All `metro-transform-plugins` suites pass: 6/6 suites, 160/160 tests, 41 snapshots. - Fresh full Flow check reports 0 errors. - Full monorepo build passes for all 17 packages. - Targeted ESLint, Prettier, and `git diff --check` pass. No public API or intended read-transform behavior changes. Reviewed By: GijsWeterings Differential Revision: D118440964 Pulled By: vzaidman fbshipit-source-id: eb4a934c1c8d469618af290356377555bcca6733 (cherry picked from commit b15a67b)
) Summary: Metro's dev server sets `Content-Type` with `mime.lookup()`, both for `/assets/` and for source files under `/[metro-project]/`. That returns a bare type like `text/html` with no charset, so clients fall back to their own default - #859 is a `WebView` rendering a UTF-8 `.html` asset as mojibake. `mime-types` has `contentType()` for this. It appends `; charset=utf-8` where a charset applies and is otherwise identical, including returning `false` for an unrecognised extension, so binary assets are unaffected. Fixes: #859 Changelog: ``` - **[Fix]**: Include `charset=utf-8` in the `Content-Type` of text assets and source files served by the dev server ``` Pull Request resolved: #1888 Test Plan: ``` yarn jest packages/metro/src/Server yarn flow check ``` New cases for a text asset, a source file and a binary file. The charset assertions fail on `main`. Reviewed By: GijsWeterings Differential Revision: D118437775 Pulled By: vzaidman fbshipit-source-id: 57a7c5809fa8638f33025b96a4aa1c90f8795b12 (cherry picked from commit eeb7934)
Summary:
Metro statically replaces `Platform.select({...})` for the target platform. Its property scan currently stops at the first matching key, while JavaScript object-literal evaluation keeps the last duplicate definition. For example, `Platform.select({ios: 1, ios: 2})` runs as `2` but Metro compiles it to `1`.
Scan the already-validated static properties from the end and return the first reverse match. This preserves O(n) behavior, avoids an unnecessary nullable temporary, and keeps compiled output consistent with runtime object semantics. The React Native Babel preset contains a parallel transform; companion [React Native PR #58249](react/react-native#58249) applies the same rule there.
Changelog:
```
- **[Fix]**: Inline the last duplicate key from static `Platform.select` object literals
```
Pull Request resolved: #1889
Test Plan:
- Added a focused `metro-transform-plugins` regression; it fails on pristine main with `1` and passes with `2` after the fix.
- All package suites pass: 6/6 suites, 160/160 tests, 41 snapshots.
- Full Flow check reports no errors.
- Full monorepo build passes for all 17 packages.
- Targeted ESLint, Prettier, and `git diff --check` pass.
No behavior changes for object literals without duplicate static keys.
Reviewed By: GijsWeterings
Differential Revision: D118499357
Pulled By: vzaidman
fbshipit-source-id: 1036c78826c4e7d12a94baa8b8eeb1a04f9f8f60
(cherry picked from commit c7bd299)
…ce RSS+heap (#1906) Summary: `FallbackWatcher`'s `recReaddir` is the only consumer of [`walker`](https://www.npmjs.com/package/walker) (unmaintained since 2021, three deps) in Metro. `walker` schedules an `lstat` for every entry the moment the parent directory is read, so a crawl has as many in-flight `fs` requests. Those requests queue against a libuv thread pool of 4, so the concurrency buys nothing and the queued request state is pure memory overhead. Note - I *don't* believe this is responsible for `EMFILE: Too many open files`, as libuv's limits should prevent that. This diff replaces it with a plain `async` `recReaddir` - a depth-first stack drained by a pool of at most 32 concurrent visits. The syscalls are identical, one `lstat` per entry and one `readdir` per directory, and so are the filters: `ignored` is still evaluated against a directory before it's descended into or reported. `endCallback` goes away in favour of awaiting, and `normalizeProxy` also goes because `path.join` already normalises. The win32 settling delay is preserved and moves up into `startWatching`, which is the only place it was used. Crawl of this repo's `node_modules`, 33,394 entries (3,160 directories, 30,054 files, 180 symlinks), median of 5 runs, macOS 25.6 on APFS, node 22.13.1: | variant | wall (ms) | RSS (MB) | peak heap (MB) | max in-flight `readdir` | |---|---|---|---|---| | `walker` | 107.2 | 102.8 | 21.7 | 818 | | bounded, 16 | 107.4 | 66.3 | 11.4 | 16 | | bounded, 32 | 103.9 | 67.5 | 12.8 | 32 | | bounded, 64 | 100.9 | 71.8 | 14.5 | 64 | | bounded, 256 | 97.2 | 85.4 | 20.2 | 239 | Both report the same 3,160/30,054/180 and issue the same number of syscalls. Wall time is constrained by the libuv pool concurrency, so 32 is chosen for the memory: **-34% RSS and -41% peak heap** against `walker`. Changelog: Internal Pull Request resolved: #1906 Test Plan: - `watchers/__tests__/integration-test.js` on all supported OSes, which drives `FallbackWatcher` against a real temporary tree - new/changed/deleted files, symlinks to a file, to a directory and to a non-existent target, pre-existing files, a directory moved in from outside the watch root, a directory moved out, and deletion of a directory with files under it. ``` yarn jest packages/metro-file-map yarn jest packages/metro/src/DeltaBundler/__tests__/resolver-test.js yarn flow check yarn lint ``` Reviewed By: GijsWeterings Differential Revision: D119076649 Pulled By: vzaidman fbshipit-source-id: f85fd675e37d8c4360d8ee2fef8e48cfdb952e81 Adapted for 0.84.x: Node.js builtins imported without the `node:` scheme. (cherry picked from commit 3173de1)
…nux + Windows) watcher (#1907) Summary: `FallbackWatcher` starts an `fs.watch` on each directory from `recReaddir`'s `dirCallback`, and the crawl calls that after `readdir` has returned. Anything written into a directory *between the two* appears in neither the listing nor the watch, so it stays invisible until the next full crawl. There's no recovery on Linux or Windows. This is expo/expo#48950 - `npx expo install` against a running dev server leaves the new modules unresolvable until restart, which is a lot more painful than it sounds now that agents routinely run Metro in a VM. brentvatne diagnosed it in expo/expo#49363, with a fix in Expo's fork that we can hopefully replace with this one. Now that we own the crawl (#1906), the fix is very simply to call `dirCallback` before `readdir` instead of after, which also moves it inside `recReaddir`'s existing `try`. Separate issues not fixed here, both pre-existing and both covered by expo/expo#49363 : - An `fs.watch` that emits `error` is never removed from `#watched`. Node emits no `close` after `error`, so the path can never be re-watched, and `#stopWatching` waits on a `close` that will not arrive. - On win32, `fs.watch` can report an event with no filename. `#detectChangedFile` drops it when `#dirRegistry[dir]` is empty, which is exactly the state a newly watched directory is in. Changelog: ``` - **[Fix]**: `FallbackWatcher` no longer misses files written to a directory while it is being crawled ``` Pull Request resolved: #1907 Test Plan: ``` yarn jest packages/metro-file-map yarn flow check yarn lint ``` New `watchers/__tests__/FallbackWatcher-test.js` asserts the ordering directly, that `fs.watch` precedes `readdir` for the same directory, on the initial crawl and on a directory created while watching, plus that a directory whose `fs.watch` throws is skipped without failing the crawl. All three fail on the parent commit and pass here. The race can't be asserted behaviourally on macOS, because FSEvents delivers with a latency window and a watch started immediately after a write still reports it: ``` $ node -e "fs.writeFileSync(d+'/raced.js', ''); fs.watch(d, (e, f) => console.log(e, f))" rename raced.js ``` That's why the issue is Linux/Windows only, and why the assertion is on the ordering rather than on a missed event. Metro has no Windows coverage for this backend, so the win32 path is unexercised either way. Reviewed By: motiz88, javache Differential Revision: D119187785 Pulled By: vzaidman fbshipit-source-id: 5b69c83db65fa546a7f4ead5e4fb75b112cad9b9 (cherry picked from commit f63b251)
…#1917) Summary: ## Context When Metro sees `require('./imgs/logo.png')` in userland source it synthesises a JS module representing `./imgs/logo.png` that encodes includes everything needed to _find_ the asset, roughly: ``` {name: "logo", ext: "png", scales: [1, 2], hash: "abc123", httpServerLocation: "/assets/imgs"} ``` In a dev server build, the client rebuilds a URL from this information and asks Metro for the file: ``` http://localhost:8081/assets/imgs/logo@2x.png?platform=ios&hash=abc123 ``` In a "release" build (more correctly - any CLI-driven build not served from the dev server, but this is typically used for (and only for) release builds), typically, frameworks use Metro's `saveAssets`, which copies each file to a **path derived from `httpServerLocation`** (even though no http server is used) at build time: ``` iOS: /assets/imgs -> assets/imgs/logo.png, assets/imgs/logo@2x.png Android: /assets/imgs -> drawable-mdpi/imgs_logo.png, drawable-xhdpi/imgs_logo.png ``` and at runtime `AssetSourceResolver` derives the lookup path from `httpServerLocation`, by the same rules. **Both halves have to agree.** `httpServerLocation` is computed twice, from two different roots. The transformer computes it per module relative to `projectRoot` and bakes it into the bundle - that's what the client uses to find an asset at runtime, in both dev and prod. The `getAssets` serialiser (for prod) computes it again over the whole graph relative to `unstable_serverRoot`, and that's what `saveAssets` uses to place files for `--assets-dest`. So if `unstable_serverRoot` is set, `Server.getAssets()` and `Server.build(<...>, {withAssets: true})` result in asset paths that do not match the JS bundle. Eg, for `unstable_serverRoot` set to a workspace root, and `projectRoot` set to the subdir `app`, the release build writes (conceptually) `assets/app/assets/logo.png` but the runtime looks for `assets/assets/logo.png`. Dev is unaffected, because there both halves of the lookup come from the transformer's copy. This dates to the original `unstable_serverRoot` change, which applied the server root broadly across `Server` and took this call site with it while both readers stayed on `projectRoot`. It was obviously never really tested - `unstable_serverRoot` is/was used at Meta but the release assets pipeline via Metro-Buck is completely different. The main motivation for this now is *not* to make it easier to use `unstable_serverRoot`, which probably should not exist, but to make asset path calculation consistent, allowing us to build on that to **fix assets outside `projectRoot`** - that's the follow up #1892. Changelog: ``` - **[Experimental]**: Fix mismatched asset paths in (release) builds under `unstable_serverRoot`, make them always relative to `projectRoot`. ``` ## Expo No immediate impact. Expo is the main setter of `unstable_serverRoot` - `expo/metro-config` points it at the workspace root. Expo already applies exactly this fix in our fork of the assets path - expo/expo#25312, since 2023. So this fix in core removes one of the reasons Expo's fork has to exist. ## Community CLI No impact - `unstable_serverRoot` (thankfully) remains completely unmentioned in RN or RNC/CLI. Pull Request resolved: #1917 Test Plan: ### Unit New test pins `_getAssetsFromDependencies` to `projectRoot` with `unstable_serverRoot` set to `/`. Restoring `_getServerRootDir()` fails it: ```console $ yarn jest packages/metro/src/Server ✕ anchors asset URLs on projectRoot, not unstable_serverRoot Tests: 1 failed, 83 passed, 84 total ``` ### End to end Built a React Native 0.87.1 app in a Yarn workspace, with an asset directory peer to the app root and `server.unstable_serverRoot` pointed at the workspace root: ```text workspace/ ├── media/ peer-logo.png, peer-logo@2x.png, peer-logo.ios.png, peer-logo@2x.ios.png └── app/ projectRoot; watchFolders: [workspaceRoot] └── assets/ in-project.png, in-project@2x.png, in-project.ios.png, in-project@2x.ios.png ``` `react-native bundle --platform ios --dev false --entry-file app/index.js --assets-dest …` on `main` produces the four-way mismatch shown under "The problem" above: the JS bundle carries `/assets/assets` and `/assets/../media` from the transformer, while the copied files land under `assets/app/assets/` and `assets/media/` from `getAssets`. Running `Metro.runBuild` with `assets: true` against the same config, at each revision, and passing the resulting `AssetData` through React Native 0.87.1's own `getAssetDestPathIOS` / `getAssetDestPathAndroid` / `filterPlatformAssetScales` - the three functions `saveAssets` uses to implement `--assets-dest`: ```text getAssets says destination main /assets/app/assets assets/app/assets/in-project.png /assets/media assets/media/peer-logo.png this PR /assets/assets assets/assets/in-project.png /assets/../media assets/_media/peer-logo.png ``` Comparing each against what the runtime derives from the bundle's copy: ```text in-project main iOS MISMATCH Android MISMATCH this PR iOS MATCH Android MATCH peer-logo main iOS MISMATCH Android MISMATCH this PR iOS MATCH Android MATCH ``` The peer asset matches without the `[metro-watchFolders]` change stacked on top, because `getAssetDestPathIOS` and `scaledAssetURLNearBundle` apply the same `../` to `_` rewrite independently. An ugly filename, but a working image. The "this PR" destinations are computed from the verified `getAssets` output fed through React Native's real helpers, rather than from a `react-native bundle` run against a patched Metro. Against a running dev server, the URLs `main` produces do not resolve at the endpoint meant to serve them, and this PR's do: ```console $ curl -o /dev/null -w '%{http_code}' '/assets/app/assets/in-project.png?platform=ios' # main 404 $ curl -o /dev/null -w '%{http_code}' '/assets/media/peer-logo.png?platform=ios' # main 404 $ curl -o /dev/null -w '%{http_code}' '/assets/assets/in-project.png?platform=ios' # this PR 200 $ curl -o /dev/null -w '%{http_code}' '/assets/../media/peer-logo.png?platform=ios' # this PR 200 ``` ### Checks Run on this commit alone, not on the stack: ```console $ yarn jest packages/metro/src Test Suites: 61 passed, 61 total Tests: 1 skipped, 869 passed, 870 total $ yarn flow check $ yarn typecheck-ts $ yarn verify-api-snapshots $ yarn lint ``` Reviewed By: GijsWeterings Differential Revision: D120129779 Pulled By: vzaidman fbshipit-source-id: 19f1dbc5e9be35f89e4a034b6e43b2ee143bf736 (cherry picked from commit 37c767e)
* Propagate transformer initialization errors * Type Bundler-test with Flow and ESM imports Adds `@flow strict-local` to the new `Bundler-test.js` and switches its `require` calls to ESM imports, matching neighboring tests such as `HmrServer-test.js`. Changelog: [Internal] --------- Co-authored-by: Rob Hogan <2590098+robhogan@users.noreply.github.com> (cherry picked from commit 1f83bfc)
…formOptions (#1757) * Fix RAM bundle passing the deps function (not its result) to getTransformOptions getRamBundleInfo gives custom getTransformOptions a callback to look up a module's transitive dependencies, but it called `Array.from(getDependencies)` on the function itself instead of `Array.from(getDependencies(x))`. Since the closure has arity 1, Array.from treated it as array-like and returned [undefined], so any getTransformOptions relying on this callback received garbage regardless of the file queried. Invoke getDependencies(x). Existing tests never exercised the callback, which masked this. Adds a regression test that calls the callback and asserts the real transitive deps (fails before / passes after). * Fix Flow error in the new getRamBundleInfo regression test `resolvedDeps` was declared without a type or initialiser, so spreading it fails `flow check` with "possibly uninitialized variable is incompatible with $Iterable". Annotate it and assert it was assigned before spreading, which also gives a clearer failure if `getTransformOptions` is never called with a dependency lookup. Changelog: [Internal] * Remove stale $FlowFixMe above the getTransformOptions dependency callback The suppression dates to Flow v0.99 in 2019 and no longer suppresses anything: `flow check` reports no error on this line with it removed, either with the fix in this stack or with the original `Array.from(getDependencies)`. Worth noting Flow did not catch this bug in either form, for the same reason it was silent at runtime - `Array.from` accepts array-likes, and a function satisfies that via `.length`. Changelog: [Internal] * Apply Prettier formatting to the regression test The guard added in the previous commit exceeded the print width, failing `prettier --check` and so `yarn lint`. Changelog: [Internal] * Share base options across the getRamBundleInfo tests Each test repeated the full serializer options object. Extract it into `baseOptions` so each test overrides only what it exercises. --------- Co-authored-by: Rob Hogan <2590098+robhogan@users.noreply.github.com> (cherry picked from commit bd4534d)
(cherry picked from commit a95869a)
Adapted for 0.84.x: Kept the `Promise<void>` return annotation added in 0.84.6. (cherry picked from commit 8751591)
`MappingsConsumer` decoded the VLQ of every mapping twice - once through its cache and once more with the result discarded - since the consumer was added in ccd508c. This removes the second call. `composeSourceMaps` with a bundle map and its Hermes map is 13.6% faster (95% CI 13.3-13.8%; 2,088ms -> 1,801ms on our benchmark app*), with byte-identical output. Composition memory is within 0.2% (-1MB*). | | Time (median) | vs previous | vs `main` | Composition memory* | vs previous | vs `main` | Peak RSS | |---|---|---|---|---|---|---|---| | `main` | 2,088ms (2,082 to 2,094) | | | 1,070MB (1,070 to 1,070) | | | 1,664MB | | This diff | 1,801ms (1,794 to 1,806) | -13.6% (-13.8 to -13.3) | -13.6% (-13.8 to -13.3) | 1,069MB (1,069 to 1,069) | -0.1% (-0.2 to -0.1) | -0.1% (-0.2 to -0.1) | 1,663MB | ## Changelog ``` - **[Performance]**: Don't decode source map segments twice in `metro-source-map`'s `Consumer` ``` ## Test plan ``` yarn jest packages/metro-source-map packages/metro-symbolicate yarn flow check ``` \* Benchmark: Mattermost Mobile 2.45.0 (React Native 0.83.9, Metro 0.83.7), iOS release bundle, unminified: 7,086 sources, 52.6MB bundle, 82.4MB flat source map. Composed with its `hermesc -O -output-source-map` map (hermes-compiler 0.14.1; 10.7MB, 1.84M segments). Timings are `composeSourceMaps` alone, excluding JSON parsing, over 105 rounds on Node 22.13.1 on an M5 Pro; each round runs `main`, every diff in this stack and a memory baseline, in a shuffled order. Figures are medians with 95% bootstrap confidence intervals - for changes, of the per-round paired ratio. Composition memory is peak RSS less that of the same process parsing both maps without composing them (594MB). (cherry picked from commit 5e08337)
* skip errors that occurred on virtual lines * Update ModuleResolution.js * Add regression test for resolution errors past the end of the source This fix has had no test since the PR was opened, which is what the review asked for in 2024. Adds one at the integration level, in `build-errors-test.js`, so it exercises the real transform and resolution path rather than constructing the error directly. `injectImportPastEndOfFileTransformer.js` wraps `@react-native/metro-babel-transformer` and, for one fixture, appends `import './does-not-exist';` parsed with `startLine` one line past the end of the source — which is what a plugin that generates and appends code does. The dependency Metro collects then reports a location with no counterpart in the file on disk, so building the code frame for the resolution error walks off the end of `lines`. Without the guard, the build rejects with `TypeError: Cannot read properties of undefined (reading 'length')` in place of the resolution error. With it, the error survives and the code frame degrades to the lines that do exist. The assertion is deliberately not a snapshot: the resolver's list of candidate paths varies between versions, and what matters here is only that the resolution error survives. Changelog: [Internal] --------- Co-authored-by: Rob Hogan <2590098+robhogan@users.noreply.github.com> (cherry picked from commit 5d6eaea)
Summary:
With `includeAsyncPaths`, Metro writes a server-relative URL for each async dependency into the `paths` argument of `__d(...)`, which the client uses to fetch the split bundle. `getDefaultAsyncDependencyPath` builds that URL with `path.relative` and `path.join`, and never converts separators, so on Windows a dependency at `C:\root\src\bar.js` under server root `C:\root` produces:
```
/src\bar.bundle?modulesOnly=true&runModule=false
```
This is a URL, and Metro's own parsing of bundle URLs treats the pathname as posix (`split('/')` in `parseBundleOptionsFromBundleRequestUrl`), so it should be `/src/bar.bundle?...` on every platform. This passes the joined path through `normalizePathSeparatorsToPosix`, already used in the same file for module debug names. No change on posix.
Changelog: [Fix] Fix backslashes in async bundle paths on Windows (`includeAsyncPaths`)
Test Plan:
Removes `js-test.js` from the Windows skip list. Its three async `paths` tests already assert forward-slash URLs from Windows-style inputs.
Before this change, on Windows CI:
```
✕ includes the paths of async dependencies when requested
- "paths":{"1":"/../bar.bundle?param1=true¶m2=1234&modulesOnly=true&runModule=false"}
+ "paths":{"1":"/..\\bar.bundle?param1=true¶m2=1234&modulesOnly=true&runModule=false"}
✕ async bundle paths override modulesOnly and runModule
✕ passes a paths object (not the id array) for async dependencies
```
After, `yarn jest packages/metro/src/DeltaBundler` passes, and Windows CI on this PR is green for `js-test.js`.
Adapted for 0.84.x: Hand-ported, since 0.84.x builds async bundle paths inline rather than in `getDefaultAsyncDependencyPath`.
(cherry picked from commit d52ee79)
Adapted for 0.84.x: Tests use 0.84.x conventions (no `node:` mocks). (cherry picked from commit 5793c17)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
robhogan
force-pushed
the
0.84/picks-for-0.84.7
branch
from
October 15, 2026 06:57
36768b6 to
2d7aa4e
Compare
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
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.
Summary
Picks the non-breaking changes from
mainsince 0.84.4 onto 0.84.x, and bumps to 0.84.7. The main motivation is #1892 (with its prerequisite #1917), so that React Native 0.86 (onmetro@^0.84.3) gets working assets outsideprojectRoot. Everything else here is a fix, perf improvement or additive feature that applies to 0.84.x without its breaking neighbours.Commits are in
mainorder, one per original commit, each with a(cherry picked from commit ...)trailer. Please don't squash. Where a pick needed adapting to 0.84.x (nonode:specifiers, nogetDefaultAsyncDependencyPathextraction, checked-in.d.ts), the commit message says so, and generated.d.tschanges are folded into the commit that caused them.This also picks #1769, which stops building and publishing
integration_tests,__fixtures__etc. Without it,yarn buildfails on theexport * asfixture added by #1768.Left out:
forceNodeFilesystemAPI, YAML config, default exports,blacklistRE, thevirtualModuleresolution type,allowOptionalDependenciesandunstable_renameRequiredefaults.FileSystemtypes inmetro-file-map, and performance: Improve metro-file-map's Node crawler hot-path #1677 / metro-file-map: Lazy plugin workers, run on demand #1950, which build on the breaking removal of the nativefindcrawler.resolver.schemeResolversand themetro:scheme (Resolver feature: schemeResolvers (#1804) #1804), which can wait for a minor.Note that #1892 changes where release builds put assets outside
projectRoot(egassets/[metro-watchFolders]/1/...on iOS,metrowatchfolders_1_...on Android). React Native derives both the copy destination and the runtime lookup fromhttpServerLocation, so they move together, but it's an observable change for a patch.Changelog:
PACKAGE_SELF_RESOLVEfor self-referencing imports in the resolver (Resolver: Implement PACKAGE_SELF_RESOLVE for self-referencing imports #1721 by @robhogan)metro-babel-transformer: PassinlinePlatformto Babel presets viacaller(6bbe095 by @robhogan)projectRoot(Use indexed watch folder paths for external assets #1892 by @robhogan)requirecalls (fix: Add ESM unwrapping to all CommonJS require calls #1675 by @kitten)metro-file-map(fix(metro-file-map): Fix Windows cross-device path resolution #1711 by @kitten)@babel/plugin-transform-modules-systemjsto fix CVE-2026-44728 (Bump @babel/plugin-transform-modules-systemjs to fix CVE-2026-44728 #1712 by @CalixTang)"browser"-spec bare-specifier -> relative path redirects for nested and first-partypackage.jsons (Resolver: Fix browser-spec bare-specifier->relative path redirect for nested/first-party package.jsons #1720 by @robhogan)"browser"-spec redirects of a bare specifier to an absolute path (e1f4d61 by @robhogan)"exports"and"imports"keys with no target falling back to a pattern (f8269d5 by @kwy404)HttpStorewrite retries incorrectly using the read endpoint's config (Fix HttpStore write retries using the read endpoint's config #1751 by @durvesh1992)HttpStore: Handle socket errors during writes, so they're retried rather than crashing the process (fad3b88)mergeConfigencounters an async config in a config chain (Fix mergeConfig dropping state when a config in the chain is async #1754 by @durvesh1992)CI=falseandCI=0as not-CI when defaultingwatch, dropping theci-infodependency (61e4592 by @robhogan)Platform.OSwrite targets during constant inlining (Preserve Platform write targets during inlining #1890 by @OskarEichler)Platform.selectobject literals (Inline the last duplicate Platform.select key #1889 by @OskarEichler)charset=utf-8in theContent-Typeof text assets and source files served by the dev server (Server: Add a charset to Content-Type for text assets and sources #1888 by @robhogan)FallbackWatcherno longer misses files written to a directory while it is being crawled (metro-file-map: Watch directories before listing them in fallback (Linux + Windows) watcher #1907 by @robhogan)FallbackWatcher: Fix missed changes and a hang on stop (metro-file-map: Fix missed changes and a stop hang in FallbackWatcher #1967 by @robhogan)NativeWatcher: Emit events in the orderfs.watchreported them (NativeWatcher: Emit events in the order fs.watch reported them - fix flaky integration tests #1974 by @robhogan)metro-file-map: IdentifyTreeFSdirectory nodes by shape, not realm (metro-file-map: Identify TreeFS directory nodes by shape, not realm #1934 by @robhogan)Bundlertransformer initialisation errors (Propagate Bundler transformer initialization errors #1854 by @Boulea7)Bundlerinitialisation (Release workers and watchers after failed Bundler initialisation #1963 by @robhogan)getTransformOptions(Fix RAM bundle passing the deps function (not its result) to getTransformOptions #1757 by @durvesh1992)MockPluginclobbering the active mock when a non-active duplicate is removed (Fix MockPlugin clobbering the active mock when a non-active duplicate is removed #1756 by @durvesh1992)stroke-widthaswidth(Fix SVG dimension parsing treating stroke-width as width #1972 by @GabrielDumbrava)generatedMappings()for indexed source maps (ab82c6a by @kwy404)metro-symbolicate: Don't throw on stack frames with no source position (metro-symbolicate: Don't throw on stack frames with no source position #1891 by @robhogan)matchSubpathFromMainFields, improving overall resolver performance ~5% (9341463 by @robhogan)FallbackWatcher: Dropwalkerdependency, reduce crawl RSS by ~34% and peak heap by ~41% (metro-file-map: Drop the walker dependency from FallbackWatcher #1906 by @robhogan)metro-source-map'sConsumer(Source map consumer: Don't decode every source map segment twice #1978 by @robhogan)unstable_serverRoot, make them always relative toprojectRoot(Fix mismatched asset paths in builds when usingunstable_serverRoot#1917 by @robhogan)experimentalImportSupport: Support namespace re-exports, such asexport * as Name from "module"(Add namespace re-export support to import-export plugin (#1768) #1768 by @robhogan)experimentalImportSupport: Fix exports renamed by destructuring, and support exports of rest spreads (experimentalImportSupport: Fix exported names under relabelling and rest spread (#1772) #1772 by @robhogan)experimentalImportSupport: Fix precedence so explicitexport defaultand named exports win overexport * from(experimentalImportSupport: Place export-all assignments before explicit exports, fix precedence ofexport default>export * from#1773 by @robhogan)experimentalImportSupport: Evaluateexport fromdependencies in the module prelude instead of within or after the module body (Hoist re-export dependencies before module body #1774 by @robhogan)experimentalImportSupport: Fixexport * fromre-exporting the source module's default export (experimentalImportSupport: Fixexport * fromincorrectly re-exporting the default export #1777 by @robhogan)Test plan
Every commit was checked out and verified on its own (Flow,
.d.tssync, lint and prettier on the touched files, and Jest--findRelatedTests), so the branch bisects cleanly. Full Jest, Flow,tscand lint pass at the tip.The watcher integration tests are timing-sensitive under heavy parallel load, on 0.84.x too - running
--findRelatedTestsforFallbackWatcher.jssix times locally, 0.84.x failed 2/6 (NativeWatchercases) and this branch 3/6 (including the new #1907 test for a directory created while watching).metro-file-map's watcher tests on their own passed 8/8 at the tip.#1892 was tested end-to-end on
main(see its test plan), and the picked version is unchanged apart from its tests.🤖 Generated with Claude Code