Skip to content

Commit

Permalink
Decouple isTesting Platform flag from disabling/enabling animations w…
Browse files Browse the repository at this point in the history
…hen testing (#38490)

Summary:
Pull Request resolved: #38490

## Changelog
[Internal] -

The internal `Platform.isTesting` is tightly coupled to animations being disabled, which in turn can lead to subtle problems in some of the corner cases:
* Since `isTesting` is force override to `false` on JS side in non-dev builds, it means that e2e tests, that would like to use release builds, are out of luck when animation disabling is desired
* Conversely, some of the e2e tests may actually rely on animations being enabled in order to work, which means they are also out of luck if trying to test a dev build
* Finally, we have cases of hybrid builds, which are build in release on native side, but also have `__DEV__=true` on the native side

To both cover the above scenarios, but also to be backwards compatible to all the existing once, this change introduces another flag, `Platform.isDisableAnimations`.

The way it works is:
* If it's not specified, the e2e tests behaviour will be exactly the same as before, since by default `isDisableAnimations` will be true when `isTesting` is true
* If it's specified and is equal to `false`, it means that animations will be still enabled, even if `isTesting` is true (for those e2e tests that rely on animations being enabled)
* If it's specified and is equal to 'true', it means that animations will be force disabled, no matter whether we test a release or a dev build

Note that this only specifies the JS side of things, defaulting `Platform.isDisableAnimations` to "not specified" (i.e. all the tests will behave as before).

Pulling it through for different platforms is done as a separate follow-up.

Differential Revision: D47516800

fbshipit-source-id: efcb78f68f9102fec025ecce9a475c9c0bc1ecca
  • Loading branch information
rshest authored and facebook-github-bot committed Jul 18, 2023
1 parent 777934e commit 04ad34d
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/react-native/Libraries/Animated/Animated.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Platform from '../Utilities/Platform';
import AnimatedImplementation from './AnimatedImplementation';
import AnimatedMock from './AnimatedMock';

const Animated = ((Platform.isTesting
const Animated = ((Platform.isDisableAnimations
? AnimatedMock
: AnimatedImplementation): typeof AnimatedImplementation);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function configureNext(
onAnimationDidEnd?: OnAnimationDidEndCallback,
onAnimationDidFail?: OnAnimationDidFailCallback,
) {
if (Platform.isTesting) {
if (Platform.isDisableAnimations) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry';
export interface Spec extends TurboModule {
+getConstants: () => {|
isTesting: boolean,
isDisableAnimations?: boolean,
reactNativeVersion: {|
major: number,
minor: number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry';
export interface Spec extends TurboModule {
+getConstants: () => {|
isTesting: boolean,
isDisableAnimations?: boolean,
reactNativeVersion: {|
major: number,
minor: number,
Expand Down
6 changes: 6 additions & 0 deletions packages/react-native/Libraries/Utilities/Platform.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const Platform = {
// $FlowFixMe[unsafe-getters-setters]
get constants(): {|
isTesting: boolean,
isDisableAnimations?: boolean,
reactNativeVersion: {|
major: number,
minor: number,
Expand Down Expand Up @@ -61,6 +62,11 @@ const Platform = {
return false;
},
// $FlowFixMe[unsafe-getters-setters]
get isDisableAnimations(): boolean {
// $FlowFixMe[object-this-reference]
return this.constants.isDisableAnimations ?? this.isTesting;
},
// $FlowFixMe[unsafe-getters-setters]
get isTV(): boolean {
// $FlowFixMe[object-this-reference]
return this.constants.uiMode === 'tv';
Expand Down
1 change: 1 addition & 0 deletions packages/react-native/Libraries/Utilities/Platform.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type PlatformOSType =
| 'native';
type PlatformConstants = {
isTesting: boolean;
isDisableAnimations?: boolean | undefined;
reactNativeVersion: {
major: number;
minor: number;
Expand Down
6 changes: 6 additions & 0 deletions packages/react-native/Libraries/Utilities/Platform.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const Platform = {
forceTouchAvailable: boolean,
interfaceIdiom: string,
isTesting: boolean,
isDisableAnimations?: boolean,
osVersion: string,
reactNativeVersion: {|
major: number,
Expand Down Expand Up @@ -65,6 +66,11 @@ const Platform = {
}
return false;
},
// $FlowFixMe[unsafe-getters-setters]
get isDisableAnimations(): boolean {
// $FlowFixMe[object-this-reference]
return this.constants.isDisableAnimations ?? this.isTesting;
},
select: <T>(spec: PlatformSelectSpec<T>): T =>
// $FlowFixMe[incompatible-return]
'ios' in spec ? spec.ios : 'native' in spec ? spec.native : spec.default,
Expand Down

0 comments on commit 04ad34d

Please sign in to comment.