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

Prepare to break the Dependency Cycle between React-graphics and React-Fabric #36385

Closed
wants to merge 1 commit into from

Conversation

cipolleschi
Copy link
Contributor

Summary:
This change moves the graphics/conversions.h files from ReactCommon/react/renderer/graphics to ReactCommon/react/renderer/core, renaming it to graphicsConversions.h.

This is required because React-Fabric imports graphics, but graphics imports React-Fabric due to this file.

The change would be breaking, but we don't want to break the ecosystem without even a warning. So, we put back the conversions.h file, which now just include the new one and outputs a warning when building. This actually maintain the dep cycle for the current version, but at least users are warned.

Changelog:

[iOS][Deprecated] - Deprecate the ReactCommon/react/renderer/graphics/conversions.h in favor of ReactCommon/react/core/graphicsConversions.h

Reviewed By: dmytrorykun

Differential Revision: D43836261

…t-Fabric

Summary:
This change moves the `graphics/conversions.h` files from `ReactCommon/react/renderer/graphics` to `ReactCommon/react/renderer/core`, renaming it to `graphicsConversions.h`.

This is required because React-Fabric imports graphics, but graphics imports React-Fabric due to this file.

The change would be breaking, but we don't want to break the ecosystem without even a warning. So, we put back the `conversions.h` file, which now just `include` the new one and outputs a warning when building. This actually maintain the dep cycle for the current version, but at least users are warned.

## Changelog:
[iOS][Deprecated] - Deprecate the `ReactCommon/react/renderer/graphics/conversions.h` in favor of `ReactCommon/react/core/graphicsConversions.h`

Reviewed By: dmytrorykun

Differential Revision: D43836261

fbshipit-source-id: d9090f6ab0b3037c3e1cd2c3dbcf549d2aa40e84
@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 6, 2023
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,509,817 +23
android hermes armeabi-v7a 7,823,819 +54
android hermes x86 8,987,351 -19
android hermes x86_64 8,843,561 +13
android jsc arm64-v8a 9,136,797 +25
android jsc armeabi-v7a 8,326,372 +60
android jsc x86 9,189,702 -14
android jsc x86_64 9,448,718 +8

Base commit: d00c150
Branch: main

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

This pull request has been merged in d72697c.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…t-Fabric (facebook#36385)

Summary:
Pull Request resolved: facebook#36385

This change moves the `graphics/conversions.h` files from `ReactCommon/react/renderer/graphics` to `ReactCommon/react/renderer/core`, renaming it to `graphicsConversions.h`.

This is required because React-Fabric imports graphics, but graphics imports React-Fabric due to this file.

The change would be breaking, but we don't want to break the ecosystem without even a warning. So, we put back the `conversions.h` file, which now just `include` the new one and outputs a warning when building. This actually maintain the dep cycle for the current version, but at least users are warned.

## Changelog:
[iOS][Deprecated] - Deprecate the `ReactCommon/react/renderer/graphics/conversions.h` in favor of `ReactCommon/react/core/graphicsConversions.h`

Reviewed By: cortinico, dmytrorykun

Differential Revision: D43836261

fbshipit-source-id: ffe53a8ce2b0ea2dd1e1e5aaf6b3d3c5b57ad46d
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 Platform: iOS iOS applications. Type: Deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants