fix: lazily evaluate Platform in Settings to resolve circular dependency startup crash (#56967)#57006
Open
siddeshgawande wants to merge 1 commit into
Open
Conversation
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #56967
Summary:
This PR resolves a fatal
TypeError: Cannot read property 'OS' of undefinedstartup crash occurring due to a circular dependency loop betweenSettings.jsandPlatform.js.Previously,
Settings.jsevaluatedPlatform.OSimmediately at module load-time. If another module requiredSettingsbeforePlatformfinished initializing its export graph,Platformresolved toundefinedand crashed the environment.This change replaces top-level platform execution with a lazy proxy initializer (
getSettings()).Platform.OSand the underlying platform implementation are now only required and resolved at runtime when a consumer invokes a method for the first time, safely eliminating the race condition.Changelog:
[GENERAL] [FIXED] - Lazily evaluate Platform in Settings to prevent circular dependency startup crashes
Test Plan:
I added a dedicated test suite to
Settings-test.jsverifying thatSettingscan be successfully imported even ifPlatformis undefined during initial module load execution.Ran the local Jest tests for the module, and all checks pass successfully: