Skip to content

Commit 965f234

Browse files
marklawlormarklawlor
committed
fix: animation fast refresh issue
1 parent 3b1cd57 commit 965f234

File tree

7 files changed

+107
-84
lines changed

7 files changed

+107
-84
lines changed

‎cpp/Animations.cpp‎

Lines changed: 69 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include "Animations.hpp"
22
#include "StyleResolver.hpp"
3+
#include "Effect.hpp"
34
#include <unordered_map>
4-
#include <mutex>
55

66
namespace reactnativecss::animations {
77

@@ -15,28 +15,30 @@ namespace reactnativecss::animations {
1515
// Map to store scope-specific computeds: Map<variableScope, Map<name, Computed<AnyMap>>>
1616
static std::unordered_map<std::string, std::unordered_map<std::string, std::shared_ptr<reactnativecss::Computed<std::shared_ptr<AnyMap>>>>> scopedComputeds;
1717

18-
static std::mutex keyframesMutex;
19-
2018
void setKeyframes(const std::string &name, const std::shared_ptr<AnyMap> &keyframes) {
21-
std::lock_guard<std::mutex> lock(keyframesMutex);
19+
std::shared_ptr<reactnativecss::Observable<std::shared_ptr<AnyMap>>> observable;
2220

2321
// Find or create the observable for this keyframe name
2422
auto it = keyframesObservables.find(name);
2523

2624
if (it != keyframesObservables.end()) {
27-
// Update existing observable
28-
it->second->set(keyframes);
25+
observable = it->second;
2926
} else {
3027
// Create new observable
31-
keyframesObservables[name] = reactnativecss::Observable<std::shared_ptr<AnyMap>>::create(
32-
keyframes);
28+
observable = reactnativecss::Observable<std::shared_ptr<AnyMap>>::create(keyframes);
29+
keyframesObservables[name] = observable;
30+
}
31+
32+
// Batch the update to prevent cascade during fast refresh
33+
if (observable) {
34+
reactnativecss::Effect::batch([&]() {
35+
observable->set(keyframes);
36+
});
3337
}
3438
}
3539

3640
std::shared_ptr<AnyMap> getKeyframes(const std::string &name, const std::string &variableScope,
3741
reactnativecss::Effect::GetProxy &get) {
38-
std::lock_guard<std::mutex> lock(keyframesMutex);
39-
4042
// First, ensure the Observable exists for this keyframe name
4143
auto obsIt = keyframesObservables.find(name);
4244
if (obsIt == keyframesObservables.end()) {
@@ -62,63 +64,73 @@ namespace reactnativecss::animations {
6264

6365
if (computedIt == scopeMap.end()) {
6466
// Create a new Computed that gets the AnyMap from the observable and processes it
65-
auto computed = reactnativecss::Computed<std::shared_ptr<AnyMap>>::create(
66-
[observable, variableScope](const std::shared_ptr<AnyMap> &prev,
67-
reactnativecss::Effect::GetProxy &get) {
68-
// Get the raw keyframes from the observable
69-
auto rawKeyframes = get(*observable);
70-
71-
// Create a new AnyMap to hold the resolved keyframes
72-
auto resolvedKeyframes = AnyMap::make(rawKeyframes->getMap().size());
73-
74-
// Loop over the entries of the rawKeyframes
75-
for (const auto &entry: rawKeyframes->getMap()) {
76-
const std::string &key = entry.first;
77-
const AnyValue &value = entry.second;
78-
79-
// Each value should be an AnyObject, if not skip that entry
80-
if (!std::holds_alternative<AnyObject>(value)) {
81-
continue;
67+
// Wrap in a batch to ensure the initial computation doesn't trigger cascades
68+
std::shared_ptr<reactnativecss::Computed<std::shared_ptr<AnyMap>>> computed;
69+
70+
reactnativecss::Effect::batch([&]() {
71+
computed = reactnativecss::Computed<std::shared_ptr<AnyMap>>::create(
72+
[observable, variableScope](const std::shared_ptr<AnyMap> &prev,
73+
reactnativecss::Effect::GetProxy &get) {
74+
// Get the raw keyframes from the observable
75+
auto rawKeyframes = get(*observable);
76+
77+
// If keyframes are empty, return early to avoid unnecessary processing
78+
if (rawKeyframes->getMap().empty()) {
79+
return AnyMap::make();
8280
}
8381

84-
const auto &frameMap = std::get<AnyObject>(value);
82+
// Create a new AnyMap to hold the resolved keyframes
83+
auto resolvedKeyframes = AnyMap::make(rawKeyframes->getMap().size());
8584

86-
// Create a temporary map to hold resolved frame values
87-
std::unordered_map<std::string, AnyValue> resolvedFrameMap;
88-
resolvedFrameMap.reserve(frameMap.size());
85+
// Loop over the entries of the rawKeyframes
86+
for (const auto &entry: rawKeyframes->getMap()) {
87+
const std::string &key = entry.first;
88+
const AnyValue &value = entry.second;
8989

90-
// Loop over each entry of the frame and resolve values
91-
for (const auto &frameEntry: frameMap) {
92-
const std::string &frameKey = frameEntry.first;
93-
const AnyValue &frameValue = frameEntry.second;
90+
// Each value should be an AnyObject, if not skip that entry
91+
if (!std::holds_alternative<AnyObject>(value)) {
92+
continue;
93+
}
9494

95-
// Resolve the value using StyleResolver
96-
AnyValue resolvedValue = margelo::nitro::cssnitro::StyleResolver::resolveStyle(
97-
frameValue, variableScope, get
98-
);
95+
const auto &frameMap = std::get<AnyObject>(value);
9996

100-
resolvedFrameMap[frameKey] = resolvedValue;
101-
}
97+
// Create a temporary map to hold resolved frame values
98+
std::unordered_map<std::string, AnyValue> resolvedFrameMap;
99+
resolvedFrameMap.reserve(frameMap.size());
102100

103-
// Apply style mapping to the resolved frame
104-
auto transformedFrame = margelo::nitro::cssnitro::StyleResolver::applyStyleMapping(
105-
resolvedFrameMap, variableScope, get
106-
);
101+
// Loop over each entry of the frame and resolve values
102+
for (const auto &frameEntry: frameMap) {
103+
const std::string &frameKey = frameEntry.first;
104+
const AnyValue &frameValue = frameEntry.second;
107105

108-
// Convert the transformed frame back to an AnyObject
109-
AnyObject finalFrame;
110-
for (const auto &transformedEntry: transformedFrame->getMap()) {
111-
finalFrame[transformedEntry.first] = transformedEntry.second;
112-
}
106+
// Resolve the value using StyleResolver
107+
AnyValue resolvedValue = margelo::nitro::cssnitro::StyleResolver::resolveStyle(
108+
frameValue, variableScope, get
109+
);
110+
111+
resolvedFrameMap[frameKey] = resolvedValue;
112+
}
113+
114+
// Apply style mapping to the resolved frame (don't process animations to avoid recursion)
115+
auto transformedFrame = margelo::nitro::cssnitro::StyleResolver::applyStyleMapping(
116+
resolvedFrameMap, variableScope, get, false
117+
);
118+
119+
// Convert the transformed frame back to an AnyObject
120+
AnyObject finalFrame;
121+
for (const auto &transformedEntry: transformedFrame->getMap()) {
122+
finalFrame[transformedEntry.first] = transformedEntry.second;
123+
}
113124

114-
// Set the resolved and transformed frame in the result
115-
resolvedKeyframes->setObject(key, finalFrame);
116-
}
125+
// Set the resolved and transformed frame in the result
126+
resolvedKeyframes->setObject(key, finalFrame);
127+
}
117128

118-
return resolvedKeyframes;
119-
},
120-
AnyMap::make()
121-
);
129+
return resolvedKeyframes;
130+
},
131+
AnyMap::make()
132+
);
133+
});
122134

123135
scopeMap[name] = computed;
124136
computedIt = scopeMap.find(name);
@@ -129,7 +141,6 @@ namespace reactnativecss::animations {
129141
}
130142

131143
void deleteScope(const std::string &name) {
132-
std::lock_guard<std::mutex> lock(keyframesMutex);
133144
scopedComputeds.erase(name);
134145
}
135146

‎cpp/StyleResolver.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ namespace margelo::nitro::cssnitro {
3838
std::shared_ptr<AnyMap> StyleResolver::applyStyleMapping(
3939
const std::unordered_map<std::string, AnyValue> &inputMap,
4040
const std::string &variableScope,
41-
typename reactnativecss::Effect::GetProxy &get
41+
typename reactnativecss::Effect::GetProxy &get,
42+
bool processAnimations
4243
) {
4344
static const std::unordered_set<std::string> transformProps = {
4445
"translateX", "translateY", "translateZ",
@@ -51,8 +52,8 @@ namespace margelo::nitro::cssnitro {
5152
auto anyMap = AnyMap::make(inputMap.size());
5253

5354
for (const auto &kv: inputMap) {
54-
// Handle animationName property
55-
if (kv.first == "animationName") {
55+
// Handle animationName property only if processAnimations is true
56+
if (processAnimations && kv.first == "animationName") {
5657
// animationName can be a string or a vector of strings
5758
if (std::holds_alternative<std::string>(kv.second)) {
5859
// Single animation name
@@ -80,7 +81,8 @@ namespace margelo::nitro::cssnitro {
8081
// Set animationName to the array of resolved keyframes
8182
anyMap->setArray("animationName", keyframesArray);
8283
} else {
83-
// Invalid type, don't set anything
84+
// Invalid type for animationName, just pass through as-is
85+
anyMap->setAny("animationName", kv.second);
8486
}
8587
continue;
8688
}

‎cpp/StyleResolver.hpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,14 @@ namespace margelo::nitro::cssnitro {
4444
* @param inputMap The map of style properties to process
4545
* @param variableScope The variable scope context for resolving animations
4646
* @param get The Effect::GetProxy for reactive dependencies
47+
* @param processAnimations Whether to process animationName properties (default true)
4748
* @return A new AnyMap with transform properties mapped into a transform array
4849
*/
4950
static std::shared_ptr<AnyMap> applyStyleMapping(
5051
const std::unordered_map<std::string, AnyValue> &inputMap,
5152
const std::string &variableScope,
52-
typename reactnativecss::Effect::GetProxy &get
53+
typename reactnativecss::Effect::GetProxy &get,
54+
bool processAnimations = true
5355
);
5456
};
5557

‎cpp/StyledComputedFactory.cpp‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -133,21 +133,34 @@ namespace margelo::nitro::cssnitro {
133133

134134
// Only perform these actions if this is a recompute (prev exists)
135135
if (prev != nullptr) {
136-
// Check if we should rerender before deleting prev
137-
if (shouldRerender(*next)) {
138-
(void) rerender();
136+
// Check if animations or transitions are present
137+
bool hasAnimations = false;
138+
if (next->style.has_value()) {
139+
hasAnimations = next->style.value()->contains("animationName") ||
140+
next->style.value()->contains("transitionProperty");
141+
}
142+
if (!hasAnimations && next->importantStyle.has_value()) {
143+
hasAnimations =
144+
next->importantStyle.value()->contains("animationName") ||
145+
next->importantStyle.value()->contains("transitionProperty");
139146
}
140147

141-
// Notify ShadowTreeUpdateManager of style changes in a batch
142-
reactnativecss::Effect::batch([&]() {
143-
if (next->style.has_value()) {
144-
shadowUpdatesPtr->addUpdates(componentId, next->style.value());
145-
}
146-
if (next->importantStyle.has_value()) {
147-
shadowUpdatesPtr->addUpdates(componentId,
148-
next->importantStyle.value());
149-
}
150-
});
148+
// If animations/transitions are present, or props changed, we must rerender
149+
if (hasAnimations || next->props.has_value() ||
150+
next->importantProps.has_value()) {
151+
(void) rerender();
152+
} else {
153+
// Only update shadow tree if no animations (shadow tree can't handle them)
154+
reactnativecss::Effect::batch([&]() {
155+
if (next->style.has_value()) {
156+
shadowUpdatesPtr->addUpdates(componentId, next->style.value());
157+
}
158+
if (next->importantStyle.has_value()) {
159+
shadowUpdatesPtr->addUpdates(componentId,
160+
next->importantStyle.value());
161+
}
162+
});
163+
}
151164

152165
// Now safe to delete prev
153166
delete prev;
@@ -160,11 +173,6 @@ namespace margelo::nitro::cssnitro {
160173
return computed;
161174
}
162175

163-
bool shouldRerender(const Styled &styled) {
164-
// Check if props has a value - if it does, we should rerender
165-
return styled.props.has_value();
166-
}
167-
168176
void StyledComputedFactory::processDeclarations(
169177
const auto &declarations,
170178
std::unordered_map<std::string, AnyValue> &targetStyles,

‎cpp/StyledComputedFactory.hpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,4 @@ namespace margelo::nitro::cssnitro {
5656
const std::string &containerScope,
5757
const std::vector<std::string> &validAttributeQueries);
5858

59-
bool shouldRerender(const Styled &styled);
60-
6159
} // namespace margelo::nitro::cssnitro

‎example/src/App.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ StyleRegistry.addStyleSheet({
1111
{
1212
s: specificity({ className: 1 }),
1313
v: {
14-
"my-custom-color": "red",
14+
"my-custom-color": "green",
1515
},
1616
d: [
1717
{

‎src/native/useStyled.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ export function useStyledProps(
4040
// Update the variable scope after we have retrieved the declarations, so it uses its own scope
4141
variableScope = declarations.variableScope ?? variableScope;
4242

43+
console.log("useStyled", { variableScope, containerScope });
44+
4345
const componentData = useMemo(() => {
4446
if (!className) {
4547
return {};

0 commit comments

Comments
 (0)