Skip to content

Commit 6dab5ec

Browse files
SnowingFoxclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 3f553d7 commit 6dab5ec

2 files changed

Lines changed: 133 additions & 7 deletions

File tree

packages/react-native/Libraries/Settings/Settings.js

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,45 @@
88
* @format
99
*/
1010

11-
import Platform from '../Utilities/Platform';
12-
13-
let Settings: {
11+
type SettingsStatic = {
1412
get(key: string): any,
1513
set(settings: Object): void,
1614
watchKeys(keys: string | Array<string>, callback: () => void): number,
1715
clearWatch(watchId: number): void,
1816
...
1917
};
2018

21-
if (Platform.OS === 'ios') {
22-
Settings = require('./Settings').default;
23-
} else {
24-
Settings = require('./SettingsFallback').default;
19+
let SettingsImpl: ?SettingsStatic = null;
20+
21+
function getSettings(): SettingsStatic {
22+
if (SettingsImpl != null) {
23+
return SettingsImpl;
24+
}
25+
const Platform = require('../Utilities/Platform').default;
26+
if (Platform.OS === 'ios') {
27+
SettingsImpl = require('./Settings').default;
28+
} else {
29+
SettingsImpl = require('./SettingsFallback').default;
30+
}
31+
return (SettingsImpl: SettingsStatic);
2532
}
2633

34+
const Settings: SettingsStatic = {
35+
get(key: string): any {
36+
return getSettings().get(key);
37+
},
38+
39+
set(settings: Object): void {
40+
getSettings().set(settings);
41+
},
42+
43+
watchKeys(keys: string | Array<string>, callback: () => void): number {
44+
return getSettings().watchKeys(keys, callback);
45+
},
46+
47+
clearWatch(watchId: number): void {
48+
getSettings().clearWatch(watchId);
49+
},
50+
};
51+
2752
export default Settings;
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
'use strict';
12+
13+
describe('Settings', () => {
14+
beforeEach(() => {
15+
jest.resetModules();
16+
});
17+
18+
it('should not throw due to circular dependency during Platform initialization', () => {
19+
// Intercept NativePlatformConstantsIOS (which Platform.ios requires during
20+
// load) to simulate another module requiring Settings during Platform's
21+
// initialization phase.
22+
jest.mock('../../Utilities/NativePlatformConstantsIOS', () => {
23+
// Accessing Settings while Platform is loading
24+
require('../Settings.js');
25+
return {
26+
getConstants() {
27+
return {
28+
interfaceIdiom: 'phone',
29+
isTesting: true,
30+
osVersion: '16.0',
31+
systemName: 'iOS',
32+
};
33+
},
34+
};
35+
});
36+
37+
expect(() => {
38+
require('../../Utilities/Platform');
39+
}).not.toThrow();
40+
});
41+
42+
it('defers accessing Platform until a method is first invoked', () => {
43+
let platformAccessCount = 0;
44+
jest.doMock('../../Utilities/Platform', () => ({
45+
__esModule: true,
46+
get default() {
47+
platformAccessCount++;
48+
return {OS: 'android'};
49+
},
50+
}));
51+
52+
const Settings = require('../Settings.js').default;
53+
expect(platformAccessCount).toBe(0);
54+
55+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
56+
Settings.get('any');
57+
expect(platformAccessCount).toBeGreaterThan(0);
58+
warnSpy.mockRestore();
59+
});
60+
61+
it('delegates get/set/watchKeys/clearWatch to the iOS implementation', () => {
62+
const setValues = jest.fn();
63+
jest.doMock('../../Utilities/Platform', () => ({
64+
__esModule: true,
65+
default: {OS: 'ios'},
66+
}));
67+
jest.doMock('../NativeSettingsManager', () => ({
68+
__esModule: true,
69+
default: {
70+
getConstants: () => ({settings: {existing: 'initial'}}),
71+
setValues,
72+
},
73+
}));
74+
75+
const Settings = require('../Settings.js').default;
76+
77+
expect(Settings.get('existing')).toBe('initial');
78+
Settings.set({added: 'value'});
79+
expect(Settings.get('added')).toBe('value');
80+
expect(setValues).toHaveBeenCalledWith({added: 'value'});
81+
82+
const watchId = Settings.watchKeys('key', () => {});
83+
expect(typeof watchId).toBe('number');
84+
expect(() => Settings.clearWatch(watchId)).not.toThrow();
85+
});
86+
87+
it('uses the fallback implementation on non-iOS platforms', () => {
88+
jest.doMock('../../Utilities/Platform', () => ({
89+
__esModule: true,
90+
default: {OS: 'android'},
91+
}));
92+
93+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
94+
const Settings = require('../Settings.js').default;
95+
96+
expect(Settings.get('foo')).toBeNull();
97+
expect(Settings.watchKeys('foo', () => {})).toBe(-1);
98+
expect(warnSpy).toHaveBeenCalled();
99+
warnSpy.mockRestore();
100+
});
101+
});

0 commit comments

Comments
 (0)