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

Break circular dependency between renderer/core and renderer/components/view #38637

Closed
wants to merge 1 commit into from

Conversation

rozele
Copy link
Contributor

@rozele rozele commented Jul 26, 2023

Summary:
ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Differential Revision: D47797967

@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 Jul 26, 2023
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

analysis-bot commented Jul 26, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,883,366 -8,841
android hermes armeabi-v7a 7,934,154 -8,532
android hermes x86 9,280,422 -9,617
android hermes x86_64 9,183,553 -10,345
android jsc arm64-v8a 9,473,107 -8,839
android jsc armeabi-v7a 8,416,462 -8,531
android jsc x86 9,455,863 -9,610
android jsc x86_64 9,772,074 -10,329

Base commit: 6cd1da9
Branch: main

rozele added a commit to rozele/react-native-macos that referenced this pull request Jul 26, 2023
…ts/view (facebook#38637)

Summary:
Pull Request resolved: facebook#38637

ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Please note, while this is technically a "breaking" change, as component descriptors for 3p components may have overridden this method, it's not supported because LayoutAnimation only works on View props.

## Changelog:
[General] [Breaking] - Remove interpolateProps functionality from ComponentDescriptor to fix circular dependency between react/renderer/core and react/renderer/components/view

Reviewed By: christophpurrer

Differential Revision: D47797967

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

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

…ts/view (facebook#38637)

Summary:
Pull Request resolved: facebook#38637

ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Please note, while this is technically a "breaking" change, as component descriptors for 3p components may have overridden this method, it's not supported because LayoutAnimation only works on View props.

## Changelog:
[General] [Breaking] - Remove interpolateProps functionality from ComponentDescriptor to fix circular dependency between react/renderer/core and react/renderer/components/view

Reviewed By: christophpurrer

Differential Revision: D47797967

fbshipit-source-id: 0f81cbc38c1a79a34d2cc0bfbb133a23b9cd4624
@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

This pull request was successfully merged by Eric Rozell in bae63d4.

When will my fix make it into a release? | Upcoming Releases

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

This pull request has been merged in bae63d4.

billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
…ts/view (facebook#38637)

Summary:
Pull Request resolved: facebook#38637

ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Please note, while this is technically a "breaking" change, as component descriptors for 3p components may have overridden this method, it's not supported because LayoutAnimation only works on View props.

## Changelog:
[General] [Breaking] - Remove interpolateProps functionality from ComponentDescriptor to fix circular dependency between react/renderer/core and react/renderer/components/view

Reviewed By: christophpurrer

Differential Revision: D47797967

fbshipit-source-id: 5285da7cc9de29f21ce14c96b850a3c58c579e94
billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
…ts/view (facebook#38637)

Summary:
Pull Request resolved: facebook#38637

ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Please note, while this is technically a "breaking" change, as component descriptors for 3p components may have overridden this method, it's not supported because LayoutAnimation only works on View props.

## Changelog:
[General] [Breaking] - Remove interpolateProps functionality from ComponentDescriptor to fix circular dependency between react/renderer/core and react/renderer/components/view

Reviewed By: christophpurrer

Differential Revision: D47797967

fbshipit-source-id: 5285da7cc9de29f21ce14c96b850a3c58c579e94
billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
…ts/view (facebook#38637)

Summary:
Pull Request resolved: facebook#38637

ComponentDescriptor and ConcreteComponentDescriptor expose a virtual method to interpolate props for LayoutAnimation. The implementation of this virtual method in ConcreteComponentDescriptor calls ViewPropsInterpolation, creating a circular dependency between react/renderer/core and react/renderer/components/view.

To break this circular dependency, this change lifts the props interpolation functionality out of ComponentDescriptor into LayoutAnimationKeyFrameManager.

Please note, while this is technically a "breaking" change, as component descriptors for 3p components may have overridden this method, it's not supported because LayoutAnimation only works on View props.

## Changelog:
[General] [Breaking] - Remove interpolateProps functionality from ComponentDescriptor to fix circular dependency between react/renderer/core and react/renderer/components/view

Reviewed By: christophpurrer

Differential Revision: D47797967

fbshipit-source-id: 5285da7cc9de29f21ce14c96b850a3c58c579e94
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