fix(Settings): defer Platform lookup to fix circular dependency startup crash (#56967) - #57878
Conversation
Summary: Fixes react#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>
|
Hi @SnowingFox! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary:
Fixes #56967.
Settings.jsevaluatedPlatform.OSat module load time (a top-levelif (Platform.OS === 'ios')branch after importingPlatformfrom../Utilities/Platform). When another module requiredSettingswhilePlatform.ios.js/Platform.android.jswas still mid-initialization (circular require — e.g.NativePlatformConstantsIOSbeing loaded byPlatform.ios.js), thePlatformexport object was not yet populated, soPlatform.OSdereferencedundefinedand threw:This fix defers the platform lookup:
Platformis now required lazily insidegetSettings()on the first method call, instead of at module load. The public API is unchanged (get/set/watchKeys/clearWatchstill delegate to the iOS implementation on iOS and the fallback implementation elsewhere), matching the existingSettings.d.ts.Changelog:
[GENERAL] [FIXED] - Defer Platform lookup in Settings.js to fix circular dependency startup crash
Test Plan:
Added
packages/react-native/Libraries/Settings/__tests__/Settings-test.js(none existed) with 4 tests:NativeSettingsManager),RED — on the unmodified code, the circular-dependency test fails with exactly the reported bug:
GREEN — after the fix:
Regression check — Settings + Utilities suites (Platform lives in
Libraries/Utilities):Also verified:
eslinton the changed files exits clean (0 errors).