Skip to content

Commit 0dee305

Browse files
Siddesh GawandeSiddesh Gawande
authored andcommitted
fix: lazily evaluate Platform in Settings to resolve circular dependency (#56967)
1 parent 9ac12ce commit 0dee305

2 files changed

Lines changed: 140 additions & 8 deletions

File tree

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

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,49 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @flow
7+
* @flow strict-local
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: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
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+
describe('Settings', () => {
12+
beforeEach(() => {
13+
jest.resetModules();
14+
});
15+
16+
describe('lazy Platform initialization', () => {
17+
it('does not access Platform when the module is first loaded', () => {
18+
let platformAccessCount = 0;
19+
jest.doMock('../../Utilities/Platform', () => ({
20+
__esModule: true,
21+
get default() {
22+
platformAccessCount++;
23+
return {OS: 'android'};
24+
},
25+
}));
26+
27+
require('../Settings.js');
28+
29+
expect(platformAccessCount).toBe(0);
30+
31+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
32+
const Settings = require('../Settings.js').default;
33+
Settings.get('any');
34+
expect(platformAccessCount).toBeGreaterThan(0);
35+
warnSpy.mockRestore();
36+
});
37+
38+
it('can be required when Platform is undefined during module load', () => {
39+
jest.doMock('../../Utilities/Platform', () => ({
40+
__esModule: true,
41+
default: undefined,
42+
}));
43+
44+
expect(() => {
45+
require('../Settings.js');
46+
}).not.toThrow();
47+
});
48+
});
49+
50+
describe('platform-specific implementation', () => {
51+
it('uses fallback implementation on Android', () => {
52+
jest.doMock('../../Utilities/Platform', () => ({
53+
__esModule: true,
54+
default: {OS: 'android'},
55+
}));
56+
57+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
58+
const Settings = require('../Settings.js').default;
59+
60+
expect(Settings.get('foo')).toBeNull();
61+
expect(Settings.watchKeys('foo', () => {})).toBe(-1);
62+
expect(warnSpy).toHaveBeenCalled();
63+
warnSpy.mockRestore();
64+
});
65+
66+
it('delegates get and set to the iOS implementation', () => {
67+
const setValues = jest.fn();
68+
jest.doMock('../../Utilities/Platform', () => ({
69+
__esModule: true,
70+
default: {OS: 'ios'},
71+
}));
72+
jest.doMock('../NativeSettingsManager', () => ({
73+
__esModule: true,
74+
default: {
75+
getConstants: () => ({settings: {myKey: 'initial'}}),
76+
setValues,
77+
},
78+
}));
79+
80+
const Settings = require('../Settings.js').default;
81+
82+
expect(Settings.get('myKey')).toBe('initial');
83+
Settings.set({myKey: 'updated'});
84+
expect(Settings.get('myKey')).toBe('updated');
85+
expect(setValues).toHaveBeenCalledWith({myKey: 'updated'});
86+
});
87+
88+
it('caches the platform implementation across calls', () => {
89+
let platformAccessCount = 0;
90+
jest.doMock('../../Utilities/Platform', () => ({
91+
__esModule: true,
92+
get default() {
93+
platformAccessCount++;
94+
return {OS: 'android'};
95+
},
96+
}));
97+
98+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
99+
const Settings = require('../Settings.js').default;
100+
101+
Settings.get('a');
102+
Settings.get('b');
103+
expect(platformAccessCount).toBe(1);
104+
warnSpy.mockRestore();
105+
});
106+
});
107+
});

0 commit comments

Comments
 (0)