Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warning when loading RCTUIManager in the old architecture #42733

Closed
wants to merge 1 commit into from

Conversation

cipolleschi
Copy link
Contributor

Summary:
When we fixed the race condition between A11yManager and RCTUIManager, we did it by moving the A11yManager on a background queue.
In the old architecture, this was raising a warning which our users might find confusing. Plus, that change was not aligned with what the A11yManager declared in its configuration because we are actually initializing it starting from a BG queue.

{F1405693310}

With this change we anticipate the initialization of the module in a place where:

  1. We know we are in the main queue
  2. We know we are going to need it (so it is not violating the lazy load principle)
  3. We know it is safe.

This should allow us to also remove the feature flag of RCTUIManagerDispatchAccessibilityManagerInitOntoMain because now it is safe to use the main_queue as requested by the module.

Changelog:

[iOS][Fixed] - Initialize the A11yManager in the main queue and when we need it.

Differential Revision: D53225120

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jan 30, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53225120

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Jan 30, 2024
Summary:

When we fixed the race condition between A11yManager and RCTUIManager, we did it by moving the A11yManager on a background queue.
In the old architecture, this was raising a warning which our users might find confusing. Plus, that change was not aligned with what the A11yManager declared in its configuration because we are actually initializing it starting from a BG queue.

{F1405693310}

With this change we anticipate the initialization of the module in a place where:
1. We know we are in the main queue
2. We know we are going to need it (so it is not violating the lazy load principle)
3. We know it is safe.

This should allow us to also remove the feature flag of `RCTUIManagerDispatchAccessibilityManagerInitOntoMain` because now it is safe to use the main_queue as requested by the module.

## Changelog:
[iOS][Fixed] - Initialize the A11yManager in the main queue and when we need it.

Differential Revision: D53225120
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53225120

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Jan 31, 2024
Summary:

When we fixed the race condition between A11yManager and RCTUIManager, we did it by moving the A11yManager on a background queue.
In the old architecture, this was raising a warning which our users might find confusing. Plus, that change was not aligned with what the A11yManager declared in its configuration because we are actually initializing it starting from a BG queue.

{F1405693310}

With this change we anticipate the initialization of the module in a place where:
1. We know we are in the main queue
2. We know we are going to need it (so it is not violating the lazy load principle)
3. We know it is safe.

This should allow us to also remove the feature flag of `RCTUIManagerDispatchAccessibilityManagerInitOntoMain` because now it is safe to use the main_queue as requested by the module.

## Changelog:
[iOS][Fixed] - Initialize the A11yManager in the main queue and when we need it.

Differential Revision: D53225120
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53225120

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Feb 7, 2024
Summary:

When we fixed the race condition between A11yManager and RCTUIManager, we did it by moving the A11yManager on a background queue.
In the old architecture, this was raising a warning which our users might find confusing. Plus, that change was not aligned with what the A11yManager declared in its configuration because we are actually initializing it starting from a BG queue.

{F1405693310}

With this change we anticipate the initialization of the module in a place where:
1. We know we are in the main queue
2. We know we are going to need it (so it is not violating the lazy load principle)
3. We know it is safe.

This should allow us to also remove the feature flag of `RCTUIManagerDispatchAccessibilityManagerInitOntoMain` because now it is safe to use the main_queue as requested by the module.

## Changelog:
[iOS][Fixed] - Initialize the A11yManager in the main queue and when we need it.

Differential Revision: D53225120
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53225120

Summary:

When we fixed the race condition between A11yManager and RCTUIManager, we did it by moving the A11yManager on a background queue.
In the old architecture, this was raising a warning which our users might find confusing. Plus, that change was not aligned with what the A11yManager declared in its configuration because we are actually initializing it starting from a BG queue.

{F1405693310}

With this change we anticipate the initialization of the module in a place where:
1. We know we are in the main queue
2. We know we are going to need it (so it is not violating the lazy load principle)
3. We know it is safe.

This should allow us to also remove the feature flag of `RCTUIManagerDispatchAccessibilityManagerInitOntoMain` because now it is safe to use the main_queue as requested by the module.

## Changelog:
[iOS][Fixed] - Initialize the A11yManager in the main queue and when we need it.

Reviewed By: philIip

Differential Revision: D53225120
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D53225120

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 7, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in dc38988.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants