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

'[skip ci] [RN][JS] UIManagerJSInterface: Fix getConstantsForViewManager types #42442

Closed
wants to merge 3 commits into from

Conversation

RSNara
Copy link
Contributor

@RSNara RSNara commented Jan 20, 2024

Summary:
If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:

  1. The only call-site to getConstantsForViewManager is getViewManagerConfig: PaperUIManager.js
  2. And getViewManagerConfig always passes in a non-null string.

So, let'''s just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D52628937

@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. p: Facebook Partner: Facebook Partner labels Jan 20, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…42442)

Summary:

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D52628937
RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…acebook#42442)

Summary:

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

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

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

RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…acebook#42442)

Summary:

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

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

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

RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…ger types (facebook#42442)

Summary:
Pull Request resolved: facebook#42442

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let'\''s just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D52628937

fbshipit-source-id: 2e16d79f28be4066b8e7f9ef62deb6c5802fe358
RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…acebook#42442)

Summary:

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D52628937
RSNara added a commit to RSNara/react-native that referenced this pull request Jan 20, 2024
…42442)

Summary:

If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

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

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

analysis-bot commented Jan 21, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,945,232 +47
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,332,436 -59
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: d16531e
Branch: main

@facebook-github-bot
Copy link
Contributor

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

Summary:
This API was used by the old architecture to lazily register/load components:
1. Load a ViewManager's class from the disk
2. Register the ViewManager's class with React Native

See: [RCTUIManager lazilyLoadView](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/React/Modules/RCTUIManager.m#L1546-L1591)

The new architecture **does not** support lazy loading of **legacy** modules/components.

Therefore, let's leave this API unimplemented until we decide to implement lazy loading of legacy stuff in new architecture.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D52677515
…#42208)

Summary:

I went through the [android](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java?fbclid=IwAR29Xk1KiGsxg7O2Fc7F2k4uKFmqqAbcMtsEEdaBalcGjJK7xuyYDo6X1Z8) and [ios](https://github.com/facebook/react-native/blob/main/packages/react-native/React/Modules/RCTUIManager.m?fbclid=IwAR2axOTNVuvGbnGw0_1wlnRhftx4W6K-ptTm_DtQ-eSOLFtnrYwzbjPyeYA) implementations of UIManagerModule. It turns out, the reactTag is always nonnull in all methods!

This diff updates UIManagerJSInterface accordingly.

Changelog: [General][Changed] - UIManagerModule: Make reactTags required

Reviewed By: sammy-SC

Differential Revision: D52627087
Summary:
If Flow can be trusted, getConstantsForViewManager always gets called with a non-null string:
1. The only call-site to getConstantsForViewManager is getViewManagerConfig: [PaperUIManager.js](https://github.com/facebook/react-native/blob/822bf52c29729d25b2bfb31655cf773609a9283d/packages/react-native/Libraries/ReactNative/PaperUIManager.js#L36-L80)
2. And getViewManagerConfig always passes in a non-null string.

So, let's just make the native argument type a non-nullable string.

Thoughts?

Changelog: [Internal]

Reviewed By: sammy-SC

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

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

@cortinico
Copy link
Contributor

d11dca7

@cortinico cortinico closed this Apr 2, 2024
@cortinico cortinico added the Merged This PR has been merged. label Apr 2, 2024
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.

4 participants