Inline the last duplicate Platform.select key - #1889
Closed
OskarEichler wants to merge 1 commit into
Closed
OskarEichler wants to merge 1 commit into
OskarEichler wants to merge 1 commit into
Conversation
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
Contributor
|
@OskarEichler Is this a real legitimate usecase? Could we throw in case duplicate "Platform.select" are used?
|
Collaborator
|
I don't think this fix is too complicated tbh (it's just iterating in reverse, because later props always overwrite earlier props at runtime - inlining should behave the same).
Yeah I agree it's niche, but it's a still legit bug that Metro's inlining changes the behaviour. I wouldn't to throw on valid JS - we could bail out of inlining, but detecting duplicates is just as much work as fixing the bug tbh. The version of the inline plugin in Metro is likely to go away soon though, so the corresponding PR to RN is more important. |
Contributor
|
@vzaidman has imported this pull request. If you are a Meta employee, you can view this in D118499357. |
Contributor
meta-codesync Bot
pushed a commit
that referenced
this pull request
Sep 11, 2026
Summary: Publish Metro 0.87.1 from `main`. Changelog: [Internal] Pull Request resolved: #1926 Test Plan: ``` node scripts/updateVersion.js 0.87.1 sl diff --stat ``` Only the 17 `packages/*/package.json` manifests change, and every `0.87.0` reference is replaced. Draft release notes (from the 91 commits in v0.87.0...main - reverted and internal-only changes omitted): ``` - **[Feature]**: Add `resolver.schemeResolvers` to resolve URI-scheme-prefixed specifiers (eg `foo:bar`) with custom resolvers (#1804 by robhogan) - **[Feature]**: Resolve `metro:babel-runtime/<path>` imports to Metro's own `babel/runtime` dependency (8388f71 by robhogan) - **[Feature]**: `metro-file-map`: Add `crawlerFactory` to optionally replace the built-in Watchman / Node crawlers (68022f0 by vzaidman) - **[Feature]**: `metro-babel-transformer`: Pass `inlinePlatform` to Babel presets via `caller` (6bbe095 by robhogan) - **[Fix]**: Replace `image-size` dependency with vendored parsers, fix CVE alerts (#1860 by robhogan) - **[Fix]**: Include `charset=utf-8` in the `Content-Type` of text assets and source files served by the dev server (#1888 by robhogan) - **[Fix]**: Preserve `Platform.OS` write targets during constant inlining (#1890 by OskarEichler) - **[Fix]**: Inline the last duplicate key from static `Platform.select` object literals (#1889 by OskarEichler) - **[Fix]**: `FallbackWatcher` no longer misses files written to a directory while it is being crawled (#1907 by robhogan) - **[Fix]**: `HttpStore`: Handle socket errors during writes, so they're retried rather than crashing the process (fad3b88) - **[Fix]**: Treat `CI=false` and `CI=0` as not-CI when defaulting `watch`. CI is now detected from `process.env.CI` only, dropping the `ci-info` dependency (61e4592 by robhogan) - **[Fix]**: Fix Fast Refresh hitting undefined modules when using lazy (segmented) bundles in dev (c70d0ae by robhogan) - **[Performance]**: `FallbackWatcher`: Drop `walker` dependency, reduce crawl RSS by ~34% and peak heap by ~41% (#1906 by robhogan) - **[Performance]**: Don't emit redundant empty dependency map arrays for modules with no dependencies (#1858 by robhogan) - **[Types]**: Restore publishing of private (underscore-prefixed) fields in TypeScript declarations (#1876 by robhogan) - **[Types]**: Fix async functions being inferred as returning `void` (not `Promise<void>`), eg `MetroServer#end` (#1909 by robhogan) - **[Types]**: Tidy up generated types, allow nullable props to be omitted in more places (#1885 by robhogan) > NOTE: Experimental features are not covered by semver and can change at any time. - **[Experimental]**: Add `serializer.unstable_inlineDependencyMap` to inline module IDs at serialisation time (#1786 by robhogan) - **[Experimental]**: Add `serializer.unstable_getAsyncDependencyPath` to supply custom `paths` to framework-defined `__loadBundleAsync` (#1855 by robhogan) - **[Experimental]**: Remove `transformer.unstable_renameRequire` - `require` is never renamed (42577f7 by robhogan) - **[Experimental]**: `experimentalImportSupport`: Fix `export * from` re-exporting the source module's default export (#1777 by robhogan) **Full Changelog**: v0.87.0...v0.87.1 ``` Reviewed By: zeyap Differential Revision: D119675927 Pulled By: GijsWeterings fbshipit-source-id: daf17f3501b4725c21c39b4dfd5c82b95e01dca5
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
3 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
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 as2but Metro compiles it to1.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 applies the same rule there.
Changelog:
Test plan
metro-transform-pluginsregression; it fails on pristine main with1and passes with2after the fix.git diff --checkpass.No behavior changes for object literals without duplicate static keys.