Skip to content

Commit 72ba44d

Browse files
committed
fix: prevent unneccessarily destroying the styled computed
1 parent 26f605c commit 72ba44d

File tree

3 files changed

+62
-30
lines changed

3 files changed

+62
-30
lines changed

README.md

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,26 @@ These are the features that are "done", in that they pass basic testing. More co
2424
- [x] Web
2525
- [x] Multiple style rules
2626
- [x] Specificity sorting
27-
- [x] Transform
28-
- [ ] Filter
2927
- [x] Pseudo classes
3028
- [x] Media query
3129
- [x] Attribute selectors
3230
- [x] Container named queries
3331
- [x] Container media queries
34-
- [ ] Container attribute queries
3532
- [x] Dynamic Variables
3633
- [x] Global variables w/ media queries
34+
- [x] Transform
35+
- [ ] Filter
36+
- [ ] Animations
37+
- [x] Transitions
38+
- [x] Important styles
39+
- [x] Important props
40+
- [ ] Safe area units
41+
- [ ] Em
42+
- [ ] `currentColor`
3743
- [ ] CSS functions (min, max, platform functions, etc)
38-
- [ ] Alt style props (e.g `headerStyle`)
3944
- [ ] Metro
4045
- [ ] Update compiler to new syntax (switch tuples to objects)
41-
- [x] Upgrading elements
42-
- [ ] Animations
43-
- [x] Transitions
44-
- [ ] Important styles
45-
- [ ] Important props
4646
- [ ] Shorthand runtime styles
4747
- [ ] Native component wrappers (e.g TextInput, ScrollView, etc)
4848
- [ ] 3rd party hook (nativeStyleToProp, etc)
49-
- [ ] Safe area units
50-
- [ ] Em
51-
- [ ] `currentColor`
49+
- [ ] 3rd party Alt style props (e.g `headerStyle`)

cpp/HybridStyleRegistry.cpp

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ namespace margelo::nitro::cssnitro {
3030
// Initialize static members
3131
std::unique_ptr<ShadowTreeUpdateManager> HybridStyleRegistry::shadowUpdates_ =
3232
std::make_unique<ShadowTreeUpdateManager>();
33-
std::unordered_map<std::string, std::shared_ptr<reactnativecss::Computed<Styled *>>> HybridStyleRegistry::computedMap_;
33+
std::unordered_map<std::string, HybridStyleRegistry::ComputedEntry> HybridStyleRegistry::computedMap_;
3434
std::unordered_map<std::string, std::shared_ptr<reactnativecss::Observable<std::vector<HybridStyleRule>>>> HybridStyleRegistry::styleRuleMap_;
3535
std::atomic<uint64_t> HybridStyleRegistry::nextStyleRuleId_{1};
3636

@@ -186,25 +186,51 @@ namespace margelo::nitro::cssnitro {
186186
const std::string &variableScope,
187187
const std::string &containerScope,
188188
const std::vector<std::string> &validAttributeQueries) {
189-
if (auto existing = computedMap_.find(componentId); existing != computedMap_.end()) {
190-
if (existing->second) {
191-
existing->second->dispose();
189+
// Check if an entry exists for this component
190+
auto existing = computedMap_.find(componentId);
191+
192+
// Only recreate computed if parameters have changed or it doesn't exist
193+
bool shouldRecreate = false;
194+
if (existing == computedMap_.end()) {
195+
shouldRecreate = true;
196+
} else {
197+
// Check if any of the parameters have changed
198+
const auto &entry = existing->second;
199+
if (entry.classNames != classNames ||
200+
entry.variableScope != variableScope ||
201+
entry.containerScope != containerScope) {
202+
shouldRecreate = true;
192203
}
193-
computedMap_.erase(existing);
194204
}
195205

196-
(void) rerender;
206+
std::shared_ptr<reactnativecss::Computed<Styled *>> computed;
197207

198-
// Build computed Styled via factory
199-
auto computed = ::margelo::nitro::cssnitro::makeStyledComputed(styleRuleMap_, classNames,
200-
componentId,
201-
rerender,
202-
*shadowUpdates_,
203-
variableScope,
204-
containerScope,
205-
validAttributeQueries);
208+
if (shouldRecreate) {
209+
// Dispose old computed if it exists
210+
if (existing != computedMap_.end() && existing->second.computed) {
211+
existing->second.computed->dispose();
212+
}
206213

207-
computedMap_[componentId] = computed;
214+
// Build new computed Styled via factory
215+
computed = ::margelo::nitro::cssnitro::makeStyledComputed(styleRuleMap_, classNames,
216+
componentId,
217+
rerender,
218+
*shadowUpdates_,
219+
variableScope,
220+
containerScope,
221+
validAttributeQueries);
222+
223+
// Store the new computed with its parameters
224+
computedMap_[componentId] = ComputedEntry{
225+
computed,
226+
classNames,
227+
variableScope,
228+
containerScope
229+
};
230+
} else {
231+
// Reuse existing computed
232+
computed = existing->second.computed;
233+
}
208234

209235
// Get the value from computed - it's a Styled* that may be nullptr
210236
Styled *styledPtr = computed->get();
@@ -221,8 +247,8 @@ namespace margelo::nitro::cssnitro {
221247
void HybridStyleRegistry::deregisterComponent(const std::string &componentId) {
222248
auto it = computedMap_.find(componentId);
223249
if (it != computedMap_.end()) {
224-
if (it->second) {
225-
it->second->dispose();
250+
if (it->second.computed) {
251+
it->second.computed->dispose();
226252
}
227253
computedMap_.erase(it);
228254
}

cpp/HybridStyleRegistry.hpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,17 @@ namespace margelo::nitro::cssnitro {
8585
const jsi::Value &thisValue,
8686
const jsi::Value *args, size_t count);
8787

88+
// Struct to hold computed with its associated parameters
89+
struct ComputedEntry {
90+
std::shared_ptr<reactnativecss::Computed<Styled *>> computed;
91+
std::string classNames;
92+
std::string variableScope;
93+
std::string containerScope;
94+
};
95+
8896
// Static shared state
8997
static std::unique_ptr<ShadowTreeUpdateManager> shadowUpdates_;
90-
static std::unordered_map<std::string, std::shared_ptr<reactnativecss::Computed<Styled *>>> computedMap_;
98+
static std::unordered_map<std::string, ComputedEntry> computedMap_;
9199
static std::unordered_map<std::string, std::shared_ptr<reactnativecss::Observable<std::vector<HybridStyleRule>>>> styleRuleMap_;
92100
static std::atomic<uint64_t> nextStyleRuleId_;
93101
};

0 commit comments

Comments
 (0)