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

Memoize markdownUtils inside the commit hook #541

Merged
merged 5 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apple/MarkdownCommitHook.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include "MarkdownTextInputDecoratorShadowNode.h"
#include "RCTMarkdownUtils.h"

using namespace facebook::react;

Expand Down Expand Up @@ -35,6 +36,10 @@ class MarkdownCommitHook : public UIManagerCommitHook {
RootShadowNode::Unshared const &newRootShadowNode) noexcept override;

private:
static RCTMarkdownUtils *
getMarkdownUtils(const MarkdownTextInputDecoratorShadowNode &decorator);
static RCTMarkdownUtils *getOrCreateMarkdownUtils(
const MarkdownTextInputDecoratorShadowNode &decorator);
const std::shared_ptr<UIManager> uiManager_;
};

Expand Down
102 changes: 81 additions & 21 deletions apple/MarkdownCommitHook.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
#include <React/RCTUtils.h>
#include <react/renderer/core/ComponentDescriptor.h>
#include <react/renderer/textlayoutmanager/RCTAttributedTextUtils.h>
#include <react/utils/ManagedObjectWrapper.h>

#include "MarkdownCommitHook.h"
#include "MarkdownShadowFamilyRegistry.h"
#include "RCTMarkdownStyle.h"
#include "RCTMarkdownUtils.h"

using namespace facebook::react;

Expand All @@ -24,6 +24,27 @@
uiManager_->unregisterCommitHook(*this);
}

RCTMarkdownUtils *MarkdownCommitHook::getMarkdownUtils(
const MarkdownTextInputDecoratorShadowNode &decorator) {
const auto decoratorState = std::static_pointer_cast<
const ConcreteState<MarkdownTextInputDecoratorState>>(
decorator.getState());
const auto memoizedUtils = (RCTMarkdownUtils *)unwrapManagedObject(
decoratorState->getData().markdownUtils);
return memoizedUtils;
}

RCTMarkdownUtils *MarkdownCommitHook::getOrCreateMarkdownUtils(
const MarkdownTextInputDecoratorShadowNode &decorator) {
const auto memoizedUtils = MarkdownCommitHook::getMarkdownUtils(decorator);

if (memoizedUtils != nullptr) {
return memoizedUtils;
} else {
return [[RCTMarkdownUtils alloc] init];
}
}

RootShadowNode::Unshared MarkdownCommitHook::shadowTreeWillCommit(
ShadowTree const &, RootShadowNode::Shared const &,
RootShadowNode::Unshared const &newRootShadowNode) noexcept {
Expand Down Expand Up @@ -94,6 +115,8 @@
const auto fontSizeMultiplier =
newRootShadowNode->getConcreteProps().layoutContext.fontSizeMultiplier;

RCTMarkdownUtils *usedUtils = nil;

// We only want to update the shadow node when the attributed string is
// stored by value If it's stored by pointer, the markdown formatting should
// already by applied to it, since the only source of that pointer (besides
Expand All @@ -115,7 +138,7 @@
nodes.textInput->getTag())) {
rootNode = rootNode->cloneTree(
nodes.textInput->getFamily(),
[&nodes, &textInputState, &stateData,
[&nodes, &textInputState, &stateData, &usedUtils,
fontSizeMultiplier](const ShadowNode &node) {
const auto &markdownProps = *std::static_pointer_cast<
MarkdownTextInputDecoratorViewProps const>(
Expand All @@ -132,8 +155,9 @@
// this can possibly be optimized
RCTMarkdownStyle *markdownStyle = [[RCTMarkdownStyle alloc]
initWithStruct:markdownProps.markdownStyle];
RCTMarkdownUtils *utils = [[RCTMarkdownUtils alloc] init];
[utils setMarkdownStyle:markdownStyle];
usedUtils =
MarkdownCommitHook::getOrCreateMarkdownUtils(*nodes.decorator);
[usedUtils setMarkdownStyle:markdownStyle];

// convert the attibuted string stored in state to
// NSAttributedString
Expand All @@ -142,14 +166,15 @@
stateData.attributedStringBox);

// Handles the first render, where the text stored in props is
// different than the one stored in state. The one in state is empty,
// while the one in props is passed from JS. If we don't update the
// state here, we'll end up with a one-default-line-sized text input
// different than the one stored in state. The one in state is
// empty, while the one in props is passed from JS. If we don't
// update the state here, we'll end up with a one-default-line-sized
// text input
if (textInputState.getRevision() == State::initialRevisionValue) {
auto plainStringFromState =
std::string([[nsAttributedString string] UTF8String]);
auto plainStringFromState =
std::string([[nsAttributedString string] UTF8String]);

if (plainStringFromState != textInputProps.text) {
if (plainStringFromState != textInputProps.text) {
// creates new AttributedString from props, adapted from
// TextInputShadowNode (ios one, text inputs are
// platform-specific)
Expand All @@ -164,14 +189,15 @@

// convert the newly created attributed string to
// NSAttributedString
nsAttributedString = RCTNSAttributedStringFromAttributedStringBox(
AttributedStringBox{attributedString});
}
nsAttributedString =
RCTNSAttributedStringFromAttributedStringBox(
AttributedStringBox{attributedString});
}
}

// apply markdown
auto newString = [utils parseMarkdown:nsAttributedString
withAttributes:defaultNSTextAttributes];
auto newString = [usedUtils parseMarkdown:nsAttributedString
withAttributes:defaultNSTextAttributes];

// create a clone of the old TextInputState and update the
// attributed string box to point to the string with markdown
Expand All @@ -187,10 +213,10 @@
});
});
} else if (stateData.attributedStringBox.getMode() ==
AttributedStringBox::Mode::OpaquePointer) {
AttributedStringBox::Mode::OpaquePointer) {
rootNode = rootNode->cloneTree(
nodes.textInput->getFamily(),
[&nodes, &textInputState, &stateData,
[&nodes, &textInputState, &stateData, &usedUtils,
fontSizeMultiplier](const ShadowNode &node) {
const auto &markdownProps = *std::static_pointer_cast<
MarkdownTextInputDecoratorViewProps const>(
Expand All @@ -207,8 +233,9 @@
// this can possibly be optimized
RCTMarkdownStyle *markdownStyle = [[RCTMarkdownStyle alloc]
initWithStruct:markdownProps.markdownStyle];
RCTMarkdownUtils *utils = [[RCTMarkdownUtils alloc] init];
[utils setMarkdownStyle:markdownStyle];
usedUtils =
MarkdownCommitHook::getOrCreateMarkdownUtils(*nodes.decorator);
[usedUtils setMarkdownStyle:markdownStyle];

// convert the attibuted string stored in state to
// NSAttributedString
Expand All @@ -217,8 +244,8 @@
stateData.attributedStringBox);

// apply markdown
auto newString = [utils parseMarkdown:nsAttributedString
withAttributes:defaultNSTextAttributes];
auto newString = [usedUtils parseMarkdown:nsAttributedString
withAttributes:defaultNSTextAttributes];

// create a clone of the old TextInputState and update the
// attributed string box to point to the string with markdown
Expand All @@ -234,6 +261,39 @@
});
});
}

// if usedUtils is not nil, then we did some work on the TextInput
// ShadowNode, we may need to update the decorator as well
if (usedUtils != nil) {
const auto ancestors =
nodes.decorator->getFamily().getAncestors(*rootNode);
const auto parentInfo = ancestors.back();
const auto decoratorNode =
std::static_pointer_cast<const MarkdownTextInputDecoratorShadowNode>(
parentInfo.first.get().getChildren().at(parentInfo.second));

// if usedUtils is defferent from the one kept in the decorator state, it
// needs to be updated to ensure memoization works correctly
if (usedUtils != MarkdownCommitHook::getMarkdownUtils(*decoratorNode)) {
const auto oldDecoratorState = *std::static_pointer_cast<
const ConcreteState<MarkdownTextInputDecoratorState>>(
decoratorNode->getState());
const auto newDecoratorState =
std::make_shared<const MarkdownTextInputDecoratorState>(
oldDecoratorState.getData().decoratorFamily,
wrapManagedObject(usedUtils));
const auto newDecoratorNode = decoratorNode->clone(
{.state = std::make_shared<
const ConcreteState<MarkdownTextInputDecoratorState>>(
newDecoratorState, oldDecoratorState)});

// since we did clone the path to the text input, parent node is mutable
// at this point - no need to clone the entire path
const auto mutableParent =
const_cast<ShadowNode *>(&parentInfo.first.get());
mutableParent->replaceChild(*decoratorNode, newDecoratorNode);
}
}
}

return std::static_pointer_cast<RootShadowNode>(rootNode);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <memory>
#include <react/renderer/core/ShadowNodeFamily.h>

namespace facebook {
Expand All @@ -9,18 +10,25 @@ class JSI_EXPORT MarkdownTextInputDecoratorState final {
public:
using Shared = std::shared_ptr<const MarkdownTextInputDecoratorState>;

MarkdownTextInputDecoratorState() : decoratorFamily(nullptr){};
MarkdownTextInputDecoratorState()
: decoratorFamily(nullptr), markdownUtils(nullptr){};
MarkdownTextInputDecoratorState(
const ShadowNodeFamily::Shared textInputFamily_)
: decoratorFamily(textInputFamily_){};
: decoratorFamily(textInputFamily_), markdownUtils(nullptr){};
MarkdownTextInputDecoratorState(
const ShadowNodeFamily::Shared textInputFamily_,
const std::shared_ptr<void> markdownUtils_)
: decoratorFamily(textInputFamily_), markdownUtils(markdownUtils_){};

#ifdef ANDROID
MarkdownTextInputDecoratorState(
MarkdownTextInputDecoratorState const &previousState, folly::dynamic data)
: decoratorFamily(previousState.decoratorFamily){};
: decoratorFamily(previousState.decoratorFamily),
markdownUtils(nullptr){};
#endif

const ShadowNodeFamily::Shared decoratorFamily;
const std::shared_ptr<void> markdownUtils;

#ifdef ANDROID
folly::dynamic getDynamic() const {
Expand Down
8 changes: 4 additions & 4 deletions example/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ PODS:
- React-logger (= 0.75.2)
- React-perflogger (= 0.75.2)
- React-utils (= 0.75.2)
- RNLiveMarkdown (0.1.169):
- RNLiveMarkdown (0.1.184):
- DoubleConversion
- glog
- hermes-engine
Expand All @@ -1517,9 +1517,9 @@ PODS:
- ReactCodegen
- ReactCommon/turbomodule/bridging
- ReactCommon/turbomodule/core
- RNLiveMarkdown/newarch (= 0.1.169)
- RNLiveMarkdown/newarch (= 0.1.184)
- Yoga
- RNLiveMarkdown/newarch (0.1.169):
- RNLiveMarkdown/newarch (0.1.184):
- DoubleConversion
- glog
- hermes-engine
Expand Down Expand Up @@ -1805,7 +1805,7 @@ SPEC CHECKSUMS:
React-utils: 81a715d9c0a2a49047e77a86f3a2247408540deb
ReactCodegen: 60973d382704c793c605b9be0fc7f31cb279442f
ReactCommon: 6ef348087d250257c44c0204461c03f036650e9b
RNLiveMarkdown: 00ab78496be2ae15a15a83f14ba732c01624f02c
RNLiveMarkdown: d5a311813de4429b9f18e764badc5bd7923cbaea
SocketRocket: abac6f5de4d4d62d24e11868d7a2f427e0ef940d
Yoga: 2a45d7e59592db061217551fd3bbe2dd993817ae

Expand Down
Loading