Skip to content

Commit b87344e

Browse files
mfkrausefacebook-github-bot
authored andcommitted
fix(babel-preset): preserve Platform.select initializers (#58350)
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(), }); ``` 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 a regression test and ran existing tests, linter and formatter. Reviewed By: GijsWeterings Differential Revision: D119065685 Pulled By: vzaidman
1 parent fea820f commit b87344e

2 files changed

Lines changed: 27 additions & 5 deletions

File tree

packages/react-native-babel-preset/src/__tests__/inline-platform-plugin-test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,16 @@ describe('Platform.select', () => {
483483
expect(select('{ios: 1, ios: 2}')).toContain('const value=2');
484484
});
485485

486+
test('does not discard impure initializers', () => {
487+
expectUnchanged(`
488+
const value = require('react-native').Platform.select({
489+
ios: first(),
490+
android: android(),
491+
ios: last(),
492+
});
493+
`);
494+
});
495+
486496
test('does not inline computed keys', () => {
487497
expect(select('{[key]: 1, default: 2}')).toContain('Platform.select');
488498
});

packages/react-native-babel-preset/src/inline-platform-plugin.js

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -520,13 +520,25 @@ module.exports = function inlinePlatformPlugin(
520520
return;
521521
}
522522

523-
path.replaceWith(
524-
findProperty(spec, platform, () =>
525-
findProperty(spec, 'native', () =>
526-
findProperty(spec, 'default', () => t.identifier('undefined')),
527-
),
523+
const replacement = findProperty(spec, platform, () =>
524+
findProperty(spec, 'native', () =>
525+
findProperty(spec, 'default', () => t.identifier('undefined')),
528526
),
529527
);
528+
// Inlining must not drop side effects from discarded property values.
529+
// Assess the property itself: an ObjectMethod has no `.value`, so
530+
// checking `property.value` would wrongly treat every method as
531+
// impure and skip inlining.
532+
if (
533+
spec.properties.every(
534+
property =>
535+
(t.isObjectProperty(property) &&
536+
property.value === replacement) ||
537+
path.scope.isPure(property),
538+
)
539+
) {
540+
path.replaceWith(replacement);
541+
}
530542
},
531543
},
532544
};

0 commit comments

Comments
 (0)