From 6dab5ec25009b6a14652406d4d2b04892d06c2a3 Mon Sep 17 00:00:00 2001 From: snowingfox Date: Mon, 10 Aug 2026 12:14:32 +0000 Subject: [PATCH] Fix Settings.js circular dependency on Platform.OS at module load Summary: Fixes #56967. Settings.js evaluated Platform.OS at module load time, so when another module required Settings while Platform was still initializing (circular require), the Platform export was undefined and reading Platform.OS threw "Cannot read properties of undefined (reading 'OS')". Defer the platform lookup: resolve Platform lazily inside getSettings() on first method call, preserving the existing iOS vs fallback behavior for get/set/watchKeys/clearWatch. Changelog: [GENERAL] [FIXED] - Defer Platform lookup in Settings.js to fix circular dependency startup crash Test Plan: - yarn jest packages/react-native/Libraries/Settings/__tests__/Settings-test.js --maxWorkers=2 RED: circular-dependency test failed with "TypeError: Cannot read properties of undefined (reading 'OS')" at Settings.js:21 GREEN: all 4 tests pass (circular init, lazy Platform access, iOS delegation, fallback) - yarn jest packages/react-native/Libraries/Settings packages/react-native/Libraries/Utilities --maxWorkers=2 15 tests pass, no regressions Co-Authored-By: Claude --- .../Libraries/Settings/Settings.js | 39 +++++-- .../Settings/__tests__/Settings-test.js | 101 ++++++++++++++++++ 2 files changed, 133 insertions(+), 7 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..881176ce456b 100644 --- a/packages/react-native/Libraries/Settings/Settings.js +++ b/packages/react-native/Libraries/Settings/Settings.js @@ -8,9 +8,7 @@ * @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..c24d007b14e8 --- /dev/null +++ b/packages/react-native/Libraries/Settings/__tests__/Settings-test.js @@ -0,0 +1,101 @@ +/** + * 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 + */ + +'use strict'; + +describe('Settings', () => { + beforeEach(() => { + jest.resetModules(); + }); + + it('should not throw due to circular dependency during Platform initialization', () => { + // Intercept NativePlatformConstantsIOS (which Platform.ios requires during + // load) to simulate another module requiring Settings during Platform's + // initialization phase. + jest.mock('../../Utilities/NativePlatformConstantsIOS', () => { + // Accessing Settings while Platform is loading + require('../Settings.js'); + return { + getConstants() { + return { + interfaceIdiom: 'phone', + isTesting: true, + osVersion: '16.0', + systemName: 'iOS', + }; + }, + }; + }); + + expect(() => { + require('../../Utilities/Platform'); + }).not.toThrow(); + }); + + it('defers accessing Platform until a method is first invoked', () => { + let platformAccessCount = 0; + jest.doMock('../../Utilities/Platform', () => ({ + __esModule: true, + get default() { + platformAccessCount++; + return {OS: 'android'}; + }, + })); + + const Settings = require('../Settings.js').default; + expect(platformAccessCount).toBe(0); + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + Settings.get('any'); + expect(platformAccessCount).toBeGreaterThan(0); + warnSpy.mockRestore(); + }); + + it('delegates get/set/watchKeys/clearWatch 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: {existing: 'initial'}}), + setValues, + }, + })); + + const Settings = require('../Settings.js').default; + + expect(Settings.get('existing')).toBe('initial'); + Settings.set({added: 'value'}); + expect(Settings.get('added')).toBe('value'); + expect(setValues).toHaveBeenCalledWith({added: 'value'}); + + const watchId = Settings.watchKeys('key', () => {}); + expect(typeof watchId).toBe('number'); + expect(() => Settings.clearWatch(watchId)).not.toThrow(); + }); + + it('uses the fallback implementation on non-iOS platforms', () => { + 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(); + }); +});