Skip to content

Commit

Permalink
chore: ios fix race condition at component registry; normalize androi…
Browse files Browse the repository at this point in the history
…d uniqueid prop
  • Loading branch information
azimgd committed Nov 22, 2024
1 parent fd9a9d0 commit 3b14bed
Show file tree
Hide file tree
Showing 14 changed files with 84 additions and 52 deletions.
5 changes: 4 additions & 1 deletion android/src/main/java/com/shadowlist/SLContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,10 @@ public void setStateWrapper(

mContainerChildrenManager.mount(
visibleStartIndex,
visibleEndIndex);
visibleEndIndex,
stateMapBuffer.getString(SLContainerManager.SLCONTAINER_STATE_FIRST_CHILD_UNIQUE_ID),
stateMapBuffer.getString(SLContainerManager.SLCONTAINER_STATE_LAST_CHILD_UNIQUE_ID)
);

float[] scrollPosition = new float[]{
(float) stateMapBuffer.getDouble(SLContainerManager.SLCONTAINER_STATE_SCROLL_POSITION_TOP),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ public void unmountChildComponentView(View childComponentView, String uniqueId)
mChildrenPool.remove(uniqueId);
}

public void mount(int visibleStartIndex, int visibleEndIndex) {
public void mount(int visibleStartIndex, int visibleEndIndex, String firstChildUniquId, String lastChildUniqueId) {
List<String> mounted = new ArrayList<>();

if (!mChildrenPool.containsKey(firstChildUniquId) || !mChildrenPool.containsKey(lastChildUniqueId)) {
return;
}

for (Map.Entry<String, View> entry : mChildrenPool.entrySet()) {
SLElement childComponentView = (SLElement) entry.getValue();

Expand Down
2 changes: 2 additions & 0 deletions android/src/main/java/com/shadowlist/SLContainerManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public class SLContainerManager extends ViewGroupManager<SLContainer>
public static final short SLCONTAINER_STATE_SCROLL_CONTAINER_HEIGHT = 9;
public static final short SLCONTAINER_STATE_HORIZONTAL = 10;
public static final short SLCONTAINER_STATE_INITIAL_NUM_TO_RENDER = 11;
public static final short SLCONTAINER_STATE_FIRST_CHILD_UNIQUE_ID = 12;
public static final short SLCONTAINER_STATE_LAST_CHILD_UNIQUE_ID = 13;

private final ViewManagerDelegate<SLContainer> mDelegate;
private OnVisibleChangeHandler mVisibleChangeHandler = null;
Expand Down
10 changes: 0 additions & 10 deletions android/src/main/java/com/shadowlist/SLElementManager.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
package com.shadowlist;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.common.MapBuilder;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.uimanager.ReactStylesDiffMap;
import com.facebook.react.uimanager.StateWrapper;
import com.facebook.react.uimanager.UIManagerHelper;
import com.facebook.react.uimanager.ViewGroupManager;
import com.facebook.react.uimanager.ThemedReactContext;
import com.facebook.react.uimanager.ViewManagerDelegate;
import com.facebook.react.uimanager.annotations.ReactProp;
import com.facebook.react.common.mapbuffer.MapBuffer;
import com.facebook.react.uimanager.events.EventDispatcher;
import com.facebook.react.viewmanagers.SLElementManagerInterface;
import com.facebook.react.viewmanagers.SLElementManagerDelegate;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ Point SLContainerShadowNode::calculateScrollPosition(const ConcreteStateData pre

Size SLContainerShadowNode::calculateScrollContent(const ConcreteStateData prevStateData, const ConcreteStateData nextStateData) {
auto &props = getConcreteProps();

Size contentSize;
auto headerFooter = 0;
auto &headerChildNode = yogaNodeFromContext(yogaNode_.getChild(0));
auto &footerChildNode = yogaNodeFromContext(yogaNode_.getChild(yogaNode_.getChildCount() - 1));

if (props.horizontal) {
contentSize = Size{nextStateData.calculateContentSize(), getLayoutMetrics().frame.size.height};
headerFooter += headerChildNode.getLayoutMetrics().frame.size.width;
Expand All @@ -143,7 +143,7 @@ Size SLContainerShadowNode::calculateScrollContent(const ConcreteStateData prevS
headerFooter += headerChildNode.getLayoutMetrics().frame.size.height;
headerFooter += footerChildNode.getLayoutMetrics().frame.size.height;
}

return contentSize;
}

Expand All @@ -153,21 +153,29 @@ std::string SLContainerShadowNode::calculateFirstChildUniqueId(const ConcreteSta
auto &childNodeViewProps = *std::static_pointer_cast<SLElementProps const>(childNode.getProps());

#ifdef ANDROID
return std::to_string(childNode.getTag());
try {
return childNode.getProps()->rawProps.at("uniqueId").asString();
} catch (...) {
return "";
}
#endif

return childNodeViewProps.uniqueId;
}

std::string SLContainerShadowNode::calculateLastChildUniqueId(const ConcreteStateData prevStateData, const ConcreteStateData nextStateData) {
// Assumes that FooterListComponent is always present, for now.
auto &childNode = yogaNodeFromContext(yogaNode_.getChild(yogaNode_.getChildCount() - 2));
auto &childNodeViewProps = *std::static_pointer_cast<SLElementProps const>(childNode.getProps());

#ifdef ANDROID
return std::to_string(childNode.getTag());
try {
return childNode.getProps()->rawProps.at("uniqueId").asString();
} catch (...) {
return "";
}
#endif

return childNodeViewProps.uniqueId;
}

Expand All @@ -179,8 +187,4 @@ YogaLayoutableShadowNode& SLContainerShadowNode::yogaNodeFromContext(YGNodeConst
return dynamic_cast<YogaLayoutableShadowNode&>(*static_cast<ShadowNode*>(YGNodeGetContext(yogaNode)));
}

SLElementShadowNode& SLContainerShadowNode::elementNodeFromContext(YGNodeConstRef yogaNode) {
return dynamic_cast<SLElementShadowNode&>(*static_cast<ShadowNode*>(YGNodeGetContext(yogaNode)));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class SLContainerShadowNode final : public ConcreteViewShadowNode<
std::string calculateFirstChildUniqueId(const ConcreteStateData prevStateData, const ConcreteStateData nextStateData);
std::string calculateLastChildUniqueId(const ConcreteStateData prevStateData, const ConcreteStateData nextStateData);
YogaLayoutableShadowNode& yogaNodeFromContext(YGNodeConstRef yogaNode);
SLElementShadowNode& elementNodeFromContext(YGNodeConstRef yogaNode);
};

}
12 changes: 10 additions & 2 deletions cpp/react/renderer/components/SLContainerSpec/SLContainerState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ SLContainerState::SLContainerState(
int visibleEndIndex,
float visibleStartTrigger,
float visibleEndTrigger,
std::string lastChildUniqueId,
std::string firstChildUniqueId,
std::string lastChildUniqueId,
bool horizontal,
int initialNumToRender) :
childrenMeasurements(childrenMeasurements),
Expand All @@ -23,8 +23,8 @@ SLContainerState::SLContainerState(
visibleEndIndex(visibleEndIndex),
visibleStartTrigger(visibleStartTrigger),
visibleEndTrigger(visibleEndTrigger),
lastChildUniqueId(lastChildUniqueId),
firstChildUniqueId(firstChildUniqueId),
lastChildUniqueId(lastChildUniqueId),
horizontal(horizontal),
initialNumToRender(initialNumToRender) {}

Expand Down Expand Up @@ -66,6 +66,12 @@ folly::dynamic SLContainerState::getDynamic() const {
)(
"initialNumToRender",
initialNumToRender
)(
"firstChildUniqueId",
firstChildUniqueId
)(
"lastChildUniqueId",
lastChildUniqueId
);
}

Expand All @@ -83,6 +89,8 @@ MapBuffer SLContainerState::getMapBuffer() const {
builder.putDouble(SLCONTAINER_STATE_SCROLL_CONTAINER_HEIGHT, scrollContainer.height);
builder.putBool(SLCONTAINER_STATE_HORIZONTAL, horizontal);
builder.putInt(SLCONTAINER_STATE_INITIAL_NUM_TO_RENDER, initialNumToRender);
builder.putString(SLCONTAINER_STATE_FIRST_CHILD_UNIQUE_ID, firstChildUniqueId);
builder.putString(SLCONTAINER_STATE_LAST_CHILD_UNIQUE_ID, lastChildUniqueId);
return builder.build();
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ constexpr static MapBuffer::Key SLCONTAINER_STATE_SCROLL_CONTAINER_WIDTH = 8;
constexpr static MapBuffer::Key SLCONTAINER_STATE_SCROLL_CONTAINER_HEIGHT = 9;
constexpr static MapBuffer::Key SLCONTAINER_STATE_HORIZONTAL = 10;
constexpr static MapBuffer::Key SLCONTAINER_STATE_INITIAL_NUM_TO_RENDER = 11;
constexpr static MapBuffer::Key SLCONTAINER_STATE_FIRST_CHILD_UNIQUE_ID = 12;
constexpr static MapBuffer::Key SLCONTAINER_STATE_LAST_CHILD_UNIQUE_ID = 13;
#endif

class SLContainerState {
Expand All @@ -38,8 +40,8 @@ class SLContainerState {
int visibleEndIndex,
float visibleStartTrigger,
float visibleEndTrigger,
std::string lastChildUniqueId,
std::string firstChildUniqueId,
std::string lastChildUniqueId,
bool horizontal,
int initialNumToRender);
SLContainerState() = default;
Expand All @@ -52,8 +54,8 @@ class SLContainerState {
int visibleEndIndex;
float visibleStartTrigger;
float visibleEndTrigger;
std::string lastChildUniqueId;
std::string firstChildUniqueId;
std::string lastChildUniqueId;
bool horizontal;
int initialNumToRender;

Expand Down Expand Up @@ -86,6 +88,8 @@ class SLContainerState {
visibleEndTrigger(
calculateVisibleEndTrigger(data["scrollPositionTop"].getDouble())
),
firstChildUniqueId(previousState.firstChildUniqueId),
lastChildUniqueId(previousState.lastChildUniqueId),
horizontal(previousState.horizontal),
initialNumToRender(previousState.initialNumToRender) {};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
/**
* This code was generated by [react-native-codegen](https://www.npmjs.com/package/react-native-codegen).
*
* Do not edit this file as changes may cause incorrect behavior and will be lost
* once the code is regenerated.
*
* @generated by codegen project: GenerateComponentHObjCpp.js
*/

#import <Foundation/Foundation.h>
#import <React/RCTDefines.h>
#import <React/RCTLog.h>
Expand Down
10 changes: 10 additions & 0 deletions cpp/react/renderer/components/SLElementSpec/SLElementProps.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "SLElementProps.h"
#include <react/renderer/core/PropsParserContext.h>
#include <react/renderer/core/propsConversions.h>
#include <react/renderer/debug/debugStringConvertibleUtils.h>

namespace facebook::react {

Expand All @@ -13,4 +14,13 @@ SLElementProps::SLElementProps(
uniqueId(convertRawProp(context, rawProps, "uniqueId", sourceProps.uniqueId, {""}))
{}

#pragma mark - DebugStringConvertible

#if RN_DEBUG_STRING_CONVERTIBLE
SharedDebugStringConvertibleList SLElementProps::getDebugProps() const {
return SharedDebugStringConvertibleList{
debugStringConvertibleItem("uniqueId", uniqueId)};
}
#endif

}
8 changes: 8 additions & 0 deletions cpp/react/renderer/components/SLElementSpec/SLElementProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <react/renderer/components/view/ViewProps.h>
#include <react/renderer/core/PropsParserContext.h>
#include <react/renderer/debug/DebugStringConvertible.h>

namespace facebook::react {

Expand All @@ -13,6 +14,13 @@ class SLElementProps final : public ViewProps {
#pragma mark - Props
int index;
std::string uniqueId{};

#pragma mark - DebugStringConvertible

#if RN_DEBUG_STRING_CONVERTIBLE
SharedDebugStringConvertibleList getDebugProps() const override;
#endif

};

}
6 changes: 5 additions & 1 deletion ios/SLContainer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared &
int visibleStartIndex = nextStateData.visibleStartIndex;
int visibleEndIndex = nextStateData.visibleEndIndex;

[self->_containerChildrenManager mount:visibleStartIndex end:visibleEndIndex];
[self->_containerChildrenManager
mount:visibleStartIndex
end:visibleEndIndex
firstChildUniqueId:[NSString stringWithUTF8String:nextStateData.firstChildUniqueId.c_str()]
lastChildUniqueId:[NSString stringWithUTF8String:nextStateData.lastChildUniqueId.c_str()]];
[self->_scrollContent setContentSize:RCTCGSizeFromSize(nextStateData.scrollContent)];
[self->_scrollContent setContentOffset:RCTCGPointFromPoint(nextStateData.scrollPosition)];

Expand Down
2 changes: 1 addition & 1 deletion ios/SLContainerChildrenManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
- (instancetype)initWithContentView:(UIView *)contentView;
- (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView uniqueId:(NSString *)uniqueId index:(NSInteger)index;
- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView uniqueId:(NSString *)uniqueId index:(NSInteger)index;
- (void)mount:(int)visibleStartIndex end:(int)visibleEndIndex;
- (void)mount:(int)visibleStartIndex end:(int)visibleEndIndex firstChildUniqueId:(NSString *)firstChildUniqueId lastChildUniqueId:(NSString *)lastChildUniqueId;

@end
#endif
29 changes: 17 additions & 12 deletions ios/SLContainerChildrenManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,32 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
[self->_childrenPool removeObjectForKey:uniqueId];
}

- (void)mount:(int)visibleStartIndex end:(int)visibleEndIndex
- (void)mount:(int)visibleStartIndex end:(int)visibleEndIndex firstChildUniqueId:(NSString *)firstChildUniqueId lastChildUniqueId:(NSString *)lastChildUniqueId
{
/**
* Temporary workaround to ensure proper mounting order.
* Currently, -(void)mountChildComponentView is called before -(void)mount in the initial phase.
* However, after components are added to the tree (e.g., via setState on the JS side),
* the order of operations is reversed. This causes the childrenRegistry to attempt mounting
* components before they are actually added to the pool.
*/
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 16 * NSEC_PER_MSEC), dispatch_get_main_queue(), ^{
std::vector<std::string> mounted = {};
for (NSString *key in self->_childrenPool) {
UIView<RCTComponentViewProtocol> *childComponentView = self->_childrenPool[key];
const auto &childViewProps = *std::static_pointer_cast<SLElementProps const>([childComponentView props]);

if (childViewProps.index >= visibleStartIndex && childViewProps.index <= visibleEndIndex)
mounted.push_back(childViewProps.uniqueId);
if (!self->_childrenPool[firstChildUniqueId] || !self->_childrenPool[lastChildUniqueId]) {
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 16 * NSEC_PER_MSEC), dispatch_get_main_queue(), ^{
[self mount:visibleStartIndex end:visibleEndIndex firstChildUniqueId:firstChildUniqueId lastChildUniqueId:lastChildUniqueId];
});
return;
}

std::vector<std::string> mounted = {};
for (NSString *key in self->_childrenPool) {
UIView<RCTComponentViewProtocol> *childComponentView = self->_childrenPool[key];
const auto &childViewProps = *std::static_pointer_cast<SLElementProps const>([childComponentView props]);

if (childViewProps.index >= visibleStartIndex && childViewProps.index <= visibleEndIndex) {
mounted.push_back(childViewProps.uniqueId);
}
}

self->_childrenRegistry.mount(mounted);
});
self->_childrenRegistry.mount(mounted);
}

@end

0 comments on commit 3b14bed

Please sign in to comment.