From 4059a695ab76f8d91a9b85bc956fdbdd5ed5ebb3 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 20 Aug 2024 16:11:31 -0700 Subject: [PATCH 1/2] Appearance: Migrate to ESM Named Exports Summary: Straightforward migration of the `Appearance` module to use ESM named exports. Changelog: [Internal] Reviewed By: TheSavior Differential Revision: D61567882 --- .../Libraries/Utilities/Appearance.js | 94 +++++++++---------- .../Libraries/Utilities/DevLoadingView.js | 6 +- .../Libraries/Utilities/useColorScheme.js | 6 +- .../__snapshots__/public-api-test.js.snap | 10 +- packages/react-native/index.js | 2 +- 5 files changed, 57 insertions(+), 61 deletions(-) diff --git a/packages/react-native/Libraries/Utilities/Appearance.js b/packages/react-native/Libraries/Utilities/Appearance.js index f08588774d585c..ad4529b027c664 100644 --- a/packages/react-native/Libraries/Utilities/Appearance.js +++ b/packages/react-native/Libraries/Utilities/Appearance.js @@ -51,57 +51,55 @@ if (NativeAppearance) { ); } -module.exports = { - /** - * Note: Although color scheme is available immediately, it may change at any - * time. Any rendering logic or styles that depend on this should try to call - * this function on every render, rather than caching the value (for example, - * using inline styles rather than setting a value in a `StyleSheet`). - * - * Example: `const colorScheme = Appearance.getColorScheme();` - * - * @returns {?ColorSchemeName} Value for the color scheme preference. - */ - getColorScheme(): ?ColorSchemeName { - if (__DEV__) { - if (isAsyncDebugging) { - // Hard code light theme when using the async debugger as - // sync calls aren't supported - return 'light'; - } +/** + * Note: Although color scheme is available immediately, it may change at any + * time. Any rendering logic or styles that depend on this should try to call + * this function on every render, rather than caching the value (for example, + * using inline styles rather than setting a value in a `StyleSheet`). + * + * Example: `const colorScheme = Appearance.getColorScheme();` + * + * @returns {?ColorSchemeName} Value for the color scheme preference. + */ +export function getColorScheme(): ?ColorSchemeName { + if (__DEV__) { + if (isAsyncDebugging) { + // Hard code light theme when using the async debugger as + // sync calls aren't supported + return 'light'; } + } - // TODO: (hramos) T52919652 Use ?ColorSchemeName once codegen supports union - const nativeColorScheme: ?string = - NativeAppearance == null - ? null - : NativeAppearance.getColorScheme() || null; - invariant( - nativeColorScheme === 'dark' || - nativeColorScheme === 'light' || - nativeColorScheme == null, - "Unrecognized color scheme. Did you mean 'dark' or 'light'?", - ); - return nativeColorScheme; - }, + // TODO: (hramos) T52919652 Use ?ColorSchemeName once codegen supports union + const nativeColorScheme: ?string = + NativeAppearance == null ? null : NativeAppearance.getColorScheme() || null; + invariant( + nativeColorScheme === 'dark' || + nativeColorScheme === 'light' || + nativeColorScheme == null, + "Unrecognized color scheme. Did you mean 'dark' or 'light'?", + ); + return nativeColorScheme; +} - setColorScheme(colorScheme: ?ColorSchemeName): void { - const nativeColorScheme = colorScheme == null ? 'unspecified' : colorScheme; +export function setColorScheme(colorScheme: ?ColorSchemeName): void { + const nativeColorScheme = colorScheme == null ? 'unspecified' : colorScheme; - invariant( - colorScheme === 'dark' || colorScheme === 'light' || colorScheme == null, - "Unrecognized color scheme. Did you mean 'dark', 'light' or null?", - ); + invariant( + colorScheme === 'dark' || colorScheme === 'light' || colorScheme == null, + "Unrecognized color scheme. Did you mean 'dark', 'light' or null?", + ); - if (NativeAppearance != null && NativeAppearance.setColorScheme != null) { - NativeAppearance.setColorScheme(nativeColorScheme); - } - }, + if (NativeAppearance != null && NativeAppearance.setColorScheme != null) { + NativeAppearance.setColorScheme(nativeColorScheme); + } +} - /** - * Add an event handler that is fired when appearance preferences change. - */ - addChangeListener(listener: AppearanceListener): EventSubscription { - return eventEmitter.addListener('change', listener); - }, -}; +/** + * Add an event handler that is fired when appearance preferences change. + */ +export function addChangeListener( + listener: AppearanceListener, +): EventSubscription { + return eventEmitter.addListener('change', listener); +} diff --git a/packages/react-native/Libraries/Utilities/DevLoadingView.js b/packages/react-native/Libraries/Utilities/DevLoadingView.js index aeb216780e759a..45119f94fcc889 100644 --- a/packages/react-native/Libraries/Utilities/DevLoadingView.js +++ b/packages/react-native/Libraries/Utilities/DevLoadingView.js @@ -9,7 +9,7 @@ */ import processColor from '../StyleSheet/processColor'; -import Appearance from './Appearance'; +import {getColorScheme} from './Appearance'; import NativeDevLoadingView from './NativeDevLoadingView'; const COLOR_SCHEME = { @@ -39,9 +39,7 @@ module.exports = { showMessage(message: string, type: 'load' | 'refresh') { if (NativeDevLoadingView) { const colorScheme = - Appearance.getColorScheme() === 'dark' - ? COLOR_SCHEME.dark - : COLOR_SCHEME.default; + getColorScheme() === 'dark' ? COLOR_SCHEME.dark : COLOR_SCHEME.default; const colorSet = colorScheme[type]; diff --git a/packages/react-native/Libraries/Utilities/useColorScheme.js b/packages/react-native/Libraries/Utilities/useColorScheme.js index 180a32e03223dc..fb5c8ef153eef1 100644 --- a/packages/react-native/Libraries/Utilities/useColorScheme.js +++ b/packages/react-native/Libraries/Utilities/useColorScheme.js @@ -12,14 +12,14 @@ import type {ColorSchemeName} from './NativeAppearance'; -import Appearance from './Appearance'; +import {addChangeListener, getColorScheme} from './Appearance'; import {useSyncExternalStore} from 'react'; const subscribe = (onStoreChange: () => void) => { - const appearanceSubscription = Appearance.addChangeListener(onStoreChange); + const appearanceSubscription = addChangeListener(onStoreChange); return () => appearanceSubscription.remove(); }; export default function useColorScheme(): ?ColorSchemeName { - return useSyncExternalStore(subscribe, Appearance.getColorScheme); + return useSyncExternalStore(subscribe, getColorScheme); } diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index 4271caff8d1ca4..92e994b858e551 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -9156,11 +9156,11 @@ declare export default typeof UTFSequence; exports[`public API should not change unintentionally Libraries/Utilities/Appearance.js 1`] = ` "type AppearanceListener = (preferences: AppearancePreferences) => void; -declare module.exports: { - getColorScheme(): ?ColorSchemeName, - setColorScheme(colorScheme: ?ColorSchemeName): void, - addChangeListener(listener: AppearanceListener): EventSubscription, -}; +declare export function getColorScheme(): ?ColorSchemeName; +declare export function setColorScheme(colorScheme: ?ColorSchemeName): void; +declare export function addChangeListener( + listener: AppearanceListener +): EventSubscription; " `; diff --git a/packages/react-native/index.js b/packages/react-native/index.js index a15bd53bdb5445..fc5fbe58e63eda 100644 --- a/packages/react-native/index.js +++ b/packages/react-native/index.js @@ -82,7 +82,7 @@ import typeof StyleSheet from './Libraries/StyleSheet/StyleSheet'; import typeof Text from './Libraries/Text/Text'; import typeof * as TurboModuleRegistry from './Libraries/TurboModule/TurboModuleRegistry'; import typeof UTFSequence from './Libraries/UTFSequence'; -import typeof Appearance from './Libraries/Utilities/Appearance'; +import typeof * as Appearance from './Libraries/Utilities/Appearance'; import typeof BackHandler from './Libraries/Utilities/BackHandler'; import typeof DeviceInfo from './Libraries/Utilities/DeviceInfo'; import typeof DevSettings from './Libraries/Utilities/DevSettings'; From d9332d9e1263d52c4001e45c7a1e10cbb4e08104 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 20 Aug 2024 19:42:11 -0700 Subject: [PATCH 2/2] Appearance: Manage Native Listener Count (Android) (#46121) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46121 Updates `Appearance` on Android to supply the native module to `NativeEventEmitter` so that the native listener count can be managed like it is on iOS. This was previously required by macOS and iOS. Android and Windows also already implement: ``` interface NativeModule { addListener(eventType: string): void; removeListeners(count: number): void; } ``` So we should start passing `NativeAppearance` into the `NativeEventEmitter` constructor across all platforms. Changelog: [Internal] Reviewed By: TheSavior Differential Revision: D61567883 --- .../Libraries/Utilities/Appearance.js | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/react-native/Libraries/Utilities/Appearance.js b/packages/react-native/Libraries/Utilities/Appearance.js index ad4529b027c664..e938742a2c51ec 100644 --- a/packages/react-native/Libraries/Utilities/Appearance.js +++ b/packages/react-native/Libraries/Utilities/Appearance.js @@ -9,7 +9,6 @@ */ import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; -import Platform from '../Utilities/Platform'; import EventEmitter, { type EventSubscription, } from '../vendor/emitter/EventEmitter'; @@ -29,26 +28,17 @@ type NativeAppearanceEventDefinitions = { appearanceChanged: [AppearancePreferences], }; -if (NativeAppearance) { - const nativeEventEmitter = - new NativeEventEmitter( - // T88715063: NativeEventEmitter only used this parameter on iOS. Now it uses it on all platforms, so this code was modified automatically to preserve its behavior - // If you want to use the native module on other platforms, please remove this condition and test its behavior - Platform.OS !== 'ios' ? null : NativeAppearance, +if (NativeAppearance != null) { + new NativeEventEmitter( + NativeAppearance, + ).addListener('appearanceChanged', (newAppearance: AppearancePreferences) => { + const {colorScheme} = newAppearance; + invariant( + colorScheme === 'dark' || colorScheme === 'light' || colorScheme == null, + "Unrecognized color scheme. Did you mean 'dark' or 'light'?", ); - nativeEventEmitter.addListener( - 'appearanceChanged', - (newAppearance: AppearancePreferences) => { - const {colorScheme} = newAppearance; - invariant( - colorScheme === 'dark' || - colorScheme === 'light' || - colorScheme == null, - "Unrecognized color scheme. Did you mean 'dark' or 'light'?", - ); - eventEmitter.emit('change', {colorScheme}); - }, - ); + eventEmitter.emit('change', {colorScheme}); + }); } /**