From 0dee3059e19744246f584c7431dc460142038789 Mon Sep 17 00:00:00 2001 From: Siddesh Gawande Date: Sat, 30 May 2026 14:45:59 +0530 Subject: [PATCH] fix: lazily evaluate Platform in Settings to resolve circular dependency (#56967) --- .../Libraries/Settings/Settings.js | 41 +++++-- .../Settings/__tests__/Settings-test.js | 107 ++++++++++++++++++ 2 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 packages/react-native/Libraries/Settings/__tests__/Settings-test.js diff --git a/packages/react-native/Libraries/Settings/Settings.js b/packages/react-native/Libraries/Settings/Settings.js index b0ec5480569f..0da88c6a4959 100644 --- a/packages/react-native/Libraries/Settings/Settings.js +++ b/packages/react-native/Libraries/Settings/Settings.js @@ -4,13 +4,11 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @flow + * @flow strict-local * @format */ -import Platform from '../Utilities/Platform'; - -let Settings: { +type SettingsStatic = { get(key: string): any, set(settings: Object): void, watchKeys(keys: string | Array, callback: () => void): number, @@ -18,10 +16,37 @@ let Settings: { ... }; -if (Platform.OS === 'ios') { - Settings = require('./Settings').default; -} else { - Settings = require('./SettingsFallback').default; +let SettingsImpl: ?SettingsStatic = null; + +function getSettings(): SettingsStatic { + if (SettingsImpl != null) { + return SettingsImpl; + } + const Platform = require('../Utilities/Platform').default; + if (Platform.OS === 'ios') { + SettingsImpl = require('./Settings').default; + } else { + SettingsImpl = require('./SettingsFallback').default; + } + return (SettingsImpl: SettingsStatic); } +const Settings: SettingsStatic = { + get(key: string): any { + return getSettings().get(key); + }, + + set(settings: Object): void { + getSettings().set(settings); + }, + + watchKeys(keys: string | Array, callback: () => void): number { + return getSettings().watchKeys(keys, callback); + }, + + clearWatch(watchId: number): void { + getSettings().clearWatch(watchId); + }, +}; + export default Settings; diff --git a/packages/react-native/Libraries/Settings/__tests__/Settings-test.js b/packages/react-native/Libraries/Settings/__tests__/Settings-test.js new file mode 100644 index 000000000000..c82720011b0a --- /dev/null +++ b/packages/react-native/Libraries/Settings/__tests__/Settings-test.js @@ -0,0 +1,107 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + */ + +describe('Settings', () => { + beforeEach(() => { + jest.resetModules(); + }); + + describe('lazy Platform initialization', () => { + it('does not access Platform when the module is first loaded', () => { + let platformAccessCount = 0; + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + get default() { + platformAccessCount++; + return {OS: 'android'}; + }, + })); + + require('../Settings.js'); + + expect(platformAccessCount).toBe(0); + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const Settings = require('../Settings.js').default; + Settings.get('any'); + expect(platformAccessCount).toBeGreaterThan(0); + warnSpy.mockRestore(); + }); + + it('can be required when Platform is undefined during module load', () => { + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + default: undefined, + })); + + expect(() => { + require('../Settings.js'); + }).not.toThrow(); + }); + }); + + describe('platform-specific implementation', () => { + it('uses fallback implementation on Android', () => { + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + default: {OS: 'android'}, + })); + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const Settings = require('../Settings.js').default; + + expect(Settings.get('foo')).toBeNull(); + expect(Settings.watchKeys('foo', () => {})).toBe(-1); + expect(warnSpy).toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it('delegates get and set to the iOS implementation', () => { + const setValues = jest.fn(); + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + default: {OS: 'ios'}, + })); + jest.doMock('../NativeSettingsManager', () => ({ + __esModule: true, + default: { + getConstants: () => ({settings: {myKey: 'initial'}}), + setValues, + }, + })); + + const Settings = require('../Settings.js').default; + + expect(Settings.get('myKey')).toBe('initial'); + Settings.set({myKey: 'updated'}); + expect(Settings.get('myKey')).toBe('updated'); + expect(setValues).toHaveBeenCalledWith({myKey: 'updated'}); + }); + + it('caches the platform implementation across calls', () => { + let platformAccessCount = 0; + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + get default() { + platformAccessCount++; + return {OS: 'android'}; + }, + })); + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const Settings = require('../Settings.js').default; + + Settings.get('a'); + Settings.get('b'); + expect(platformAccessCount).toBe(1); + warnSpy.mockRestore(); + }); + }); +});