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(iOS) [0.74]: properly warn about createRootViewWithBridge #43146

Conversation

okwasniewski
Copy link
Contributor

Summary:

This PR fixes an issue that _logWarnIfCreateRootViewWithBridgeIsOverridden was called in wrong place.

Assuming user overrides this method and call to [super]:

- (UIView *)createRootViewWithBridge:(RCTBridge *)bridge moduleName:(NSString *)moduleName initProps:(NSDictionary *)initProps {
  UIView *view = [super createRootViewWithBridge:bridge moduleName:moduleName initProps:initProps];
  view.backgroundColor = [UIColor redColor];
  return view;
}

This method still wasn't called in bridgeless (and not showing the error).

Checking if user overrides this method in appDidFinishWithLaunching works every time

simulator_screenshot_0E22557C-CE37-4617-A25A-F39A6ED4D3D0

Changelog:

[IOS] [FIXED] - Properly warn about createRootViewWithBridge being deprecated

Test Plan:

Check if warning is shown when message is overridden

@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: Callstack Partner: Callstack Partner labels Feb 22, 2024
@okwasniewski
Copy link
Contributor Author

cc: @cipolleschi

@analysis-bot
Copy link

analysis-bot commented Feb 22, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,935,061 -65,622
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,293,286 -65,778
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 073ca1a
Branch: main

@cipolleschi
Copy link
Contributor

can you please add a cherry pick request in the proper discussion?

@okwasniewski
Copy link
Contributor Author

Added: reactwg/react-native-releases#104 (comment)

So I should re-target this to main?

@cipolleschi
Copy link
Contributor

probably yes.

@okwasniewski okwasniewski force-pushed the fix/properly-warn-deprecated-method branch from 3b3925a to a18663c Compare February 23, 2024 09:03
@okwasniewski okwasniewski changed the base branch from 0.74-stable to main February 23, 2024 09:03
@okwasniewski okwasniewski force-pushed the fix/properly-warn-deprecated-method branch from a18663c to 5d9b8b2 Compare February 23, 2024 09:03
@okwasniewski
Copy link
Contributor Author

@cipolleschi I've changed the target

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Feb 23, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

@cipolleschi merged this pull request in a119769.

huntie pushed a commit that referenced this pull request Mar 4, 2024
Summary:
This PR fixes an issue that `_logWarnIfCreateRootViewWithBridgeIsOverridden` was called in wrong place.

Assuming user overrides this method and call to `[super]`:

```objc
- (UIView *)createRootViewWithBridge:(RCTBridge *)bridge moduleName:(NSString *)moduleName initProps:(NSDictionary *)initProps {
  UIView *view = [super createRootViewWithBridge:bridge moduleName:moduleName initProps:initProps];
  view.backgroundColor = [UIColor redColor];
  return view;
}
```

This method still wasn't called in bridgeless (and not showing the error).

Checking if user overrides this method in `appDidFinishWithLaunching` works every time

![simulator_screenshot_0E22557C-CE37-4617-A25A-F39A6ED4D3D0](https://github.com/facebook/react-native/assets/52801365/d7865f37-32f0-40ad-a252-74ab7c5b7757)

## Changelog:

[IOS] [FIXED] - Properly warn about `createRootViewWithBridge` being deprecated

Pull Request resolved: #43146

Test Plan: Check if warning is shown when message is overridden

Reviewed By: huntie

Differential Revision: D54303506

Pulled By: cipolleschi

fbshipit-source-id: cf30555c791493f28b3015a189cf93b60cace8f8
okwasniewski added a commit to okwasniewski/react-native that referenced this pull request Mar 5, 2024
…ok#43146)

Summary:
This PR fixes an issue that `_logWarnIfCreateRootViewWithBridgeIsOverridden` was called in wrong place.

Assuming user overrides this method and call to `[super]`:

```objc
- (UIView *)createRootViewWithBridge:(RCTBridge *)bridge moduleName:(NSString *)moduleName initProps:(NSDictionary *)initProps {
  UIView *view = [super createRootViewWithBridge:bridge moduleName:moduleName initProps:initProps];
  view.backgroundColor = [UIColor redColor];
  return view;
}
```

This method still wasn't called in bridgeless (and not showing the error).

Checking if user overrides this method in `appDidFinishWithLaunching` works every time

![simulator_screenshot_0E22557C-CE37-4617-A25A-F39A6ED4D3D0](https://github.com/facebook/react-native/assets/52801365/d7865f37-32f0-40ad-a252-74ab7c5b7757)

## Changelog:

[IOS] [FIXED] - Properly warn about `createRootViewWithBridge` being deprecated

Pull Request resolved: facebook#43146

Test Plan: Check if warning is shown when message is overridden

Reviewed By: huntie

Differential Revision: D54303506

Pulled By: cipolleschi

fbshipit-source-id: cf30555c791493f28b3015a189cf93b60cace8f8
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. Merged This PR has been merged. p: Callstack Partner: Callstack Partner Pick Request Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants