Skip to content

Commit ee30313

Browse files
vzaidmanfacebook-github-bot
authored andcommitted
Revert preserve Platform.select initializers
Summary: Reverts #58350, restoring the long-standing `Platform.select` inlining behavior. Per robhogan and vonovak: Metro has inlined Platform.select() (in release builds only, not configurable) forever really - it goes back to at least 2017, before the start of the Metro repo, eg: react/metro@a317b9d It's always had that behaviour where it collapses side-effects of inactive branches. Recently, the plugin was copied/re-implemented into RN. Right now (RN 0.88, Metro 0.87) they both have their own version of the plugin and Metro's runs after RN's, but Metro's shouldn't find anything left to inline and should be a no-op. I couldn't remove it from Metro immediately only because it'd need a major Metro release, but I plan to remove it soon. So RN's is effectively a continuation of Metro's 8 year old behaviour - we just moved the implementation. Changelog: [General][Fixed] - Restore long-standing Platform.select inlining behavior that collapses side effects of inactive branches See discussion: #58442 Differential Revision: D120147924
1 parent 5ad1d5c commit ee30313

2 files changed

Lines changed: 6 additions & 39 deletions

File tree

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -483,25 +483,6 @@ 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-
496-
test('does not mutate object methods when bailing out on impure initializers', () => {
497-
expectUnchanged(`
498-
const value = require('react-native').Platform.select({
499-
ios() { return 1; },
500-
android: sideEffect(),
501-
});
502-
`);
503-
});
504-
505486
test('does not inline computed keys', () => {
506487
expect(select('{[key]: 1, default: 2}')).toContain('Platform.select');
507488
});

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -443,9 +443,7 @@ module.exports = function inlinePlatformPlugin(
443443
if (t.isObjectProperty(property)) {
444444
return property.value;
445445
}
446-
// Clone: toExpression mutates in place, e.g. `ios() {}` would be
447-
// left mutated if the purity check below bails out.
448-
return t.toExpression(t.cloneNode(property));
446+
return t.toExpression(property);
449447
}
450448
}
451449
return fallback();
@@ -522,25 +520,13 @@ module.exports = function inlinePlatformPlugin(
522520
return;
523521
}
524522

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

0 commit comments

Comments
 (0)