From 6dab5ec25009b6a14652406d4d2b04892d06c2a3 Mon Sep 17 00:00:00 2001 From: snowingfox Date: Mon, 10 Aug 2026 12:14:32 +0000 Subject: [PATCH 1/2] 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(); + }); +}); From 689e19a3c47902a1a440e61fab07b957937790c1 Mon Sep 17 00:00:00 2001 From: SnowingFox <26035718+SnowingFox@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:57:43 +0800 Subject: [PATCH 2/2] Fix Settings Flow compatibility on current main Use the supported as-cast syntax for the cached implementation. Observe lazy Platform loading in the Jest mock factory instead of a side-effectful getter, preserving both regression assertions without a Flow suppression. Validation: - Reproduced both CI Flow errors before the change. - yarn flow-check: Found 0 errors. - Settings and Utilities Jest suites: 3 suites, 15 tests passed. - Revised circular and lazy-load tests fail against main and pass here. - ESLint --max-warnings=0 and Prettier checks passed. - yarn build-types --validate passed. - Generated and legacy TypeScript consumer checks passed. Co-Authored-By: Claude Opus 5.5 --- .../react-native/Libraries/Settings/Settings.js | 2 +- .../Libraries/Settings/__tests__/Settings-test.js | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/react-native/Libraries/Settings/Settings.js b/packages/react-native/Libraries/Settings/Settings.js index b975e9947ac8..261ec4820673 100644 --- a/packages/react-native/Libraries/Settings/Settings.js +++ b/packages/react-native/Libraries/Settings/Settings.js @@ -28,7 +28,7 @@ function getSettings(): SettingsStatic { } else { SettingsImpl = require('./SettingsFallback').default; } - return (SettingsImpl: SettingsStatic); + return SettingsImpl as SettingsStatic; } const Settings: SettingsStatic = { diff --git a/packages/react-native/Libraries/Settings/__tests__/Settings-test.js b/packages/react-native/Libraries/Settings/__tests__/Settings-test.js index c24d007b14e8..06c18216f1ba 100644 --- a/packages/react-native/Libraries/Settings/__tests__/Settings-test.js +++ b/packages/react-native/Libraries/Settings/__tests__/Settings-test.js @@ -41,13 +41,13 @@ describe('Settings', () => { 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'}; - }, - })); + jest.doMock('../../Utilities/Platform', () => { + platformAccessCount++; + return { + __esModule: true, + default: {OS: 'android'}, + }; + }); const Settings = require('../Settings.js').default; expect(platformAccessCount).toBe(0);