Skip to content

fix(babel-preset): preserve Platform.select initializers (#58350) - #58442

Open
vzaidman wants to merge 1 commit into
mainfrom
export-D119484692
Open

fix(babel-preset): preserve Platform.select initializers (#58350)#58442
vzaidman wants to merge 1 commit into
mainfrom
export-D119484692

Conversation

@vzaidman

Copy link
Copy Markdown
Contributor

Summary:
The React Native Babel preset replaces Platform.select({...}) with the selected property during production transforms. JavaScript evaluates every object property initializer before calling Platform.select, so this can silently remove side effects from non-selected properties.

For example:

Platform.select({
  ios: selected(),
  android: discarded(),
});

OR

Platform.select({
  ios() {
    selected()
  },
  android: discarded(),
});

JavaScript would run both side effects, selected() and discarded(). The plugin's current behavior removes discarded on iOS however.

This fix preserves both side effects by testing for purity.

Changelog:

[GENERAL] [FIXED] - Preserve side effects from discarded Platform.select property initializers in the Babel preset.

Test Plan: Added regression tests and ran existing tests, linter and formatter. Verified the ObjectMethod bailout case emits the source unchanged instead of invalid JS.

Differential Revision: D119484692

Pulled By: vzaidman

Summary:
The React Native Babel preset replaces `Platform.select({...})` with the selected property during production transforms. JavaScript evaluates every object property initializer before calling `Platform.select`, so this can silently remove side effects from non-selected properties.

For example:

```js
Platform.select({
  ios: selected(),
  android: discarded(),
});
```

OR

```js
Platform.select({
  ios() {
    selected()
  },
  android: discarded(),
});
```

JavaScript would run both side effects, `selected()` and `discarded()`. The plugin's current behavior removes `discarded` on iOS however.

This fix preserves both side effects by testing for purity.

## Changelog:

[GENERAL] [FIXED] - Preserve side effects from discarded Platform.select property initializers in the Babel preset.


Test Plan: Added regression tests and ran existing tests, linter and formatter. Verified the ObjectMethod bailout case emits the source unchanged instead of invalid JS.

Differential Revision: D119484692

Pulled By: vzaidman
@meta-cla meta-cla 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 10, 2026
@meta-codesync

meta-codesync Bot commented Sep 10, 2026

Copy link
Copy Markdown

@vzaidman has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119484692.

@vonovak

vonovak commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

@vzaidman Hello, I'd like to push back on this one a bit.

Firstly, on the idea itself. Platform.select picks a value. IMO the arms are object values because that's how you use the api to select a value, not because they were meant to run side-effectful code.

I ran the branch through Metro to see what it actually does:

It doesn't change anything for normal app code. Metro sets inlinePlatform: true and always adds its own inline plugin. That plugin matches Platform.select as well, and it has no purity check. So when the new check bails out, the call just gets collapsed by Metro a bit later anyway. Here's Platform.select({ios: iosSide(), android: androidSide()}) built for Android with this branch applied:

dev   var v = androidSide();
prod  var v = androidSide();

Dev and prod already behave the same. Metro's plugin runs in dev too, so Platform.select is collapsed there as well. It's not something that only breaks in prod.

This affects even RN itself. Two files import Platform with a relative path, which Metro's matcher doesn't recognise, so nothing collapses them once the new check bails: SafeAreaView and nativeImageSource. For SafeAreaView that means Metro now picks up ./RCTSafeAreaViewNativeComponent as a dependency, so Android bundles and runs an iOS-only spec.

I understand the motivation, but the point of this plugin is to collapse platform branches. This check makes it quietly stop doing that for a whole class of ordinary code: any arm that is a property read, like Platform.select({ios: fonts.bold, default: fonts.medium}). scope.isPure treats every member expression as impure, since a getter could in theory do something, even though none of these typically do. The point is that the day Metro's matcher goes (its removal is the stated plan in 40c0612), the inliner starts silently giving up on code with no side effects in it at all, and nothing tells you it happened.

Both examples above show the change can grow the bundle. The RN ones are also a correctness problem: the app ends up shipping and running code meant for another platform.

So my ask is to not land this at all. Dropping the other platform's arm is the point of the inliner, not a bug. The only thing this changes today is that two RN files start shipping iOS code to Android.

Btw, Expo has been doing the unconditional version of this. It turns Metro's plugin off and uses its own, which runs in prod only and collapses Platform.select with no purity check. I'm not aware of that causing issues since it was merged in March 2024.

Thanks for your consideration!

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants