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

Audit Community Modules for Thread Affinity Issues with RNW 0.64 #6444

Closed
NickGerleman opened this issue Nov 6, 2020 · 5 comments
Closed

Comments

@NickGerleman
Copy link
Collaborator

#6284 changes semantics around native module thread affinity, where native modules are now called on the JS engine thread instead of the UI thread. Previous UI threadiness was unintentional, but likely means there are community modules using UI-thread affined resources from a native module.

We should try to provide a transition path for the break (e.g. UI-thread native modules though QuirkSettings), but may also need to audit any community modules that we are responsible for maintaining. Because these are generally lacking test coverage, it is likely we will otherwise see many of them break with 0.64.

@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Nov 6, 2020
@asklar asklar added this to the 0.64 milestone Nov 9, 2020
@asklar asklar added Area: Native Modules must-have p1 and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Nov 9, 2020
@asklar
Copy link
Member

asklar commented Nov 9, 2020

Any native module that needs to access UI elements will need to be audited. Do we have a list? ViewManagers are not affected, only Native Modules.
https://microsoft.github.io/react-native-windows/docs/supported-community-modules

@asklar
Copy link
Member

asklar commented Nov 9, 2020

Meta-point: we have breaking changes for NM across releases (C# for 0.63, e.g.). How do we deal with keeping community modules working despite breaking changes in the platform?
Breaking changes in Native Modules should have a higher bar than other breaking changes.
Do we need to make the breaking change now at all?

@ghost ghost added the Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) label Nov 9, 2020
@asklar asklar added bug and removed Invalid Triage https://github.com/microsoft/react-native-windows/wiki/Triage-Process (label applied by bot) labels Nov 9, 2020
@chrisglein
Copy link
Member

At this point I believe we've taken this change into 0.64 but not backported further than that. Now that 0.64 first preview is out, we have a clear release candidate to evaluate native module compatibility against.

@chiaramooney
Copy link
Contributor

chiaramooney commented Feb 1, 2021

See here for page list of native modules which have been added to the gallery running on 0.64. All components attached to these pages are running and working on 0.64.

react-native-webview component is blocked at the moment as a change was needed for the component to be able to be deployed in an RNW app. There is a PR out now for the update necessary. Once new version of webview is released it can be added to gallery and tested on 0.64.

react-native-progress-view is blocked at the moment. PR out now to fix build issues with module. Once new version of progress-view is released it can be added to gallery and tested on 0.64.

react-native-linear-gradient blocked at the moment. Module needs updating in order to be compatible with 0.63+. Issue created to track this task #7008.

Checked out react-native-camera to see if it is compatible with 0.64. Upgraded CameraDemo to run on 0.64. App builds without error for both debug and release. However, I couldn't get the app to find my camera. The camera was able to connect and show a preview of it's view when I was running on 0.63. Couldn't determine if it's an issue with the module or my laptop wasn't firing the appropriate "ask for camera permission" question as it did when I ran in 0.63.

Release still has not been cut for react-native-orientation-locker containing the adding Windows support PR. Blocked from checking that this module works with gallery until that happens.

@chiaramooney
Copy link
Contributor

Closing issue for now; all community modules we maintain have been reviewed and documented in above comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants