fix(babel-preset): preserve Platform.select initializers (#58350) - #58442
fix(babel-preset): preserve Platform.select initializers (#58350)#58442vzaidman wants to merge 1 commit into
Conversation
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
|
@vzaidman has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119484692. |
|
@vzaidman Hello, I'd like to push back on this one a bit. Firstly, on the idea itself. I ran the branch through Metro to see what it actually does: It doesn't change anything for normal app code. Metro sets Dev and prod already behave the same. Metro's plugin runs in dev too, so This affects even RN itself. Two files import 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 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 Thanks for your consideration! |
Summary:
The React Native Babel preset replaces
Platform.select({...})with the selected property during production transforms. JavaScript evaluates every object property initializer before callingPlatform.select, so this can silently remove side effects from non-selected properties.For example:
OR
JavaScript would run both side effects,
selected()anddiscarded(). The plugin's current behavior removesdiscardedon 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