Skip to content

Commit

Permalink
Break circular dependency between renderer/core and renderer/componen…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
rozele authored and billnbell3 committed Jul 29, 2023
1 parent ec579b9 commit c223d5b
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <react/renderer/componentregistry/ComponentDescriptorFactory.h>
#include <react/renderer/components/image/ImageProps.h>
#include <react/renderer/components/view/ViewProps.h>
#include <react/renderer/components/view/ViewPropsInterpolation.h>
#include <react/renderer/core/ComponentDescriptor.h>
#include <react/renderer/core/LayoutMetrics.h>
#include <react/renderer/core/Props.h>
Expand Down Expand Up @@ -1152,8 +1153,13 @@ ShadowView LayoutAnimationKeyFrameManager::createInterpolatedShadowView(
// Animate opacity or scale/transform
PropsParserContext propsParserContext{
finalView.surfaceId, *contextContainer_};
mutatedShadowView.props = componentDescriptor.interpolateProps(
propsParserContext, progress, startingView.props, finalView.props);
mutatedShadowView.props = interpolateProps(
componentDescriptor,
propsParserContext,
progress,
startingView.props,
finalView.props);

react_native_assert(mutatedShadowView.props != nullptr);
if (mutatedShadowView.props == nullptr) {
return finalView;
Expand Down Expand Up @@ -1656,4 +1662,32 @@ void LayoutAnimationKeyFrameManager::deleteAnimationsForStoppedSurfaces()
}
}

Props::Shared LayoutAnimationKeyFrameManager::interpolateProps(
const ComponentDescriptor &componentDescriptor,
const PropsParserContext &context,
Float animationProgress,
const Props::Shared &props,
const Props::Shared &newProps) const {
#ifdef ANDROID
// On Android only, the merged props should have the same RawProps as the
// final props struct
Props::Shared interpolatedPropsShared =
(newProps != nullptr
? componentDescriptor.cloneProps(
context, newProps, newProps->rawProps)
: componentDescriptor.cloneProps(context, newProps, {}));
#else
Props::Shared interpolatedPropsShared =
componentDescriptor.cloneProps(context, newProps, {});
#endif

if (componentDescriptor.getTraits().check(
ShadowNodeTraits::Trait::ViewKind)) {
interpolateViewProps(
animationProgress, props, newProps, interpolatedPropsShared);
}

return interpolatedPropsShared;
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ class LayoutAnimationKeyFrameManager : public UIManagerAnimationDelegate,

void simulateImagePropsMemoryAccess(
ShadowViewMutationList const &mutations) const;

/**
* Interpolates the props values.
*/
Props::Shared interpolateProps(
const ComponentDescriptor &componentDescriptor,
const PropsParserContext &context,
Float animationProgress,
const Props::Shared &props,
const Props::Shared &newProps) const;
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,6 @@ class ComponentDescriptor {
const Props::Shared &props,
const RawProps &rawProps) const = 0;

/*
* Creates a new `Props` of a particular type with all values interpolated
* between `props` and `newProps`.
*/
virtual Props::Shared interpolateProps(
const PropsParserContext &context,
Float animationProgress,
const Props::Shared &props,
const Props::Shared &newProps) const = 0;

/*
* Create an initial State object that represents (and contains) an initial
* State's data which can be constructed based on initial Props.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <memory>

#include <react/debug/react_native_assert.h>
#include <react/renderer/components/view/ViewPropsInterpolation.h>
#include <react/renderer/core/ComponentDescriptor.h>
#include <react/renderer/core/EventDispatcher.h>
#include <react/renderer/core/Props.h>
Expand Down Expand Up @@ -125,30 +124,6 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
return shadowNodeProps;
};

Props::Shared interpolateProps(
const PropsParserContext &context,
Float animationProgress,
const Props::Shared &props,
const Props::Shared &newProps) const override {
#ifdef ANDROID
// On Android only, the merged props should have the same RawProps as the
// final props struct
Props::Shared interpolatedPropsShared =
(newProps != nullptr ? cloneProps(context, newProps, newProps->rawProps)
: cloneProps(context, newProps, {}));
#else
Props::Shared interpolatedPropsShared = cloneProps(context, newProps, {});
#endif

if (ConcreteShadowNode::BaseTraits().check(
ShadowNodeTraits::Trait::ViewKind)) {
interpolateViewProps(
animationProgress, props, newProps, interpolatedPropsShared);
}

return interpolatedPropsShared;
};

virtual State::Shared createInitialState(
Props::Shared const &props,
ShadowNodeFamily::Shared const &family) const override {
Expand Down

0 comments on commit c223d5b

Please sign in to comment.