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

Forward-declare imports in Binding and FabricMountingManager #36609

Closed
wants to merge 1 commit into from

Conversation

javache
Copy link
Member

@javache javache commented Mar 23, 2023

Summary:
Some random cleanup as I prepare to make these classes a better injection point for future experiments.

  • Forward-declare classes where possible to reduce header import
  • Return references to shared_ptr instead of copies when there are no lifetime concerns
  • Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

@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 fb-exported labels Mar 23, 2023
@facebook-github-bot
Copy link
Contributor

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

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

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

javache added a commit to javache/react-native that referenced this pull request Mar 23, 2023
…k#36609)

Summary:
Pull Request resolved: facebook#36609

Some random cleanup as I prepare to make these classes a better injection point for future experiments.

* Forward-declare classes where possible to reduce header import
* Return references to shared_ptr instead of copies when there are no lifetime concerns
* Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

fbshipit-source-id: 774f77ab9827dce9b068eb7b85146409e2b08af7
@facebook-github-bot
Copy link
Contributor

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

javache added a commit to javache/react-native that referenced this pull request Mar 23, 2023
…k#36609)

Summary:
Pull Request resolved: facebook#36609

Some random cleanup as I prepare to make these classes a better injection point for future experiments.

* Forward-declare classes where possible to reduce header import
* Return references to shared_ptr instead of copies when there are no lifetime concerns
* Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

fbshipit-source-id: 383faea5efa09fd88104decec73cbc93e13d2058
…k#36609)

Summary:
Pull Request resolved: facebook#36609

Some random cleanup as I prepare to make these classes a better injection point for future experiments.

* Forward-declare classes where possible to reduce header import
* Return references to shared_ptr instead of copies when there are no lifetime concerns
* Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

fbshipit-source-id: 2cf673f6ef5cdf69fa7369c8561ba059662ddb0c
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,518,544 -1,517
android hermes armeabi-v7a 7,834,881 -1,190
android hermes x86 8,997,728 -2,304
android hermes x86_64 8,853,847 -1,388
android jsc arm64-v8a 9,139,902 -1,503
android jsc armeabi-v7a 8,332,166 -1,190
android jsc x86 9,193,606 -2,309
android jsc x86_64 9,452,646 -1,386

Base commit: 3759a26
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 23, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7259cb3.

@javache javache deleted the export-D44221018 branch March 24, 2023 10:20
jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
…k#36609)

Summary:
Pull Request resolved: facebook#36609

Some random cleanup as I prepare to make these classes a better injection point for future experiments.

* Forward-declare classes where possible to reduce header import
* Return references to shared_ptr instead of copies when there are no lifetime concerns
* Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

fbshipit-source-id: 1660cac964abd10ce798473e26841503430efdfe
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…k#36609)

Summary:
Pull Request resolved: facebook#36609

Some random cleanup as I prepare to make these classes a better injection point for future experiments.

* Forward-declare classes where possible to reduce header import
* Return references to shared_ptr instead of copies when there are no lifetime concerns
* Use a shared JClass instance in JFabricUIManager

Changelog: [Internal]

Reviewed By: rshest

Differential Revision: D44221018

fbshipit-source-id: 1660cac964abd10ce798473e26841503430efdfe
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.

3 participants