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): remove unused RCTTurboModuleManagerDelegate method #48290

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

okwasniewski
Copy link
Contributor

@okwasniewski okwasniewski commented Dec 16, 2024

Summary:

Hey, this PR removes unused method from RCTAppDelegate.

The only called method from RCTTurboModuleManagerDelegate is getTurboModule:jsInvoker, the getTurboModule:initParams: is never called.

return [_appTMMDelegate getTurboModule:name jsInvoker:jsInvoker];

Changelog:

[INTERNAL] [REMOVED] - remove unused RCTTurboModuleManagerDelegate method

Test Plan:

CI Green, ensure everything works as before

@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 Dec 16, 2024
@okwasniewski
Copy link
Contributor Author

cc: @cipolleschi

@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 Dec 16, 2024
@cipolleschi
Copy link
Contributor

I'd rather keep the method. This is the method that we use to load pure C++ turbomodule. Although it is "empty" it can be useful as a reference for people... WDYT?

@okwasniewski
Copy link
Contributor Author

okwasniewski commented Dec 16, 2024

@cipolleschi Can you point me to a place where its called / which protocol requires us to implement it? I think this is the method you are referring to:

- (std::shared_ptr<TurboModule>)getTurboModule:(const std::string &)name
(which users can actually override)

When I place a breakpoint in this method its never called

@okwasniewski
Copy link
Contributor Author

@cipolleschi I agree that if it's called then then let's leave it but I think it's not actually called which leads to not being the best example for users 😅

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. p: Callstack Partner: Callstack Partner 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.

3 participants