Conversation
Member
|
Are there still to do items for this? Curious why it's a draft PR. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
After the Expo 57 updates in #451, I witnessed a ~33% increase in Uniwind's mass-render benchmark. It turns out that to satisfy Strict Mode guarantees (i.e. effects getting destroyed and then recreated in dev-only mode, which is to support use cases that e.g.
<Activity />provide), on the first mount, we were now running styles and rules updates twice -- even if we weren't in dev mode (which makes sense to have done to properly fix the issue, because<Activity />exists).Fix
This PR makes one overarching change comprised of two back-to-back changes (and adds some regression tests) to resolve this:
updateRules. The effect still re-runs and re-calculates if thestyleEffectorruleEffect's change (i.e. previous/existing behavior).<Activity mode="hidden" />-- also recalculate the rules.More Details
LLM summary (TODO benchmark values need fixing -- improvement closer to 33%)
Fix NativeWind 5 rc.0 performance regression: optimize fresh-mount reconnect
Problem
NativeWind 5 (react-native-css rc.0) is ~42% slower than NativeWind 4 in bulk remount benchmarks (~426ms vs ~200ms). PR #451's unconditional reconnect pattern runs a full
updateRulespass and forces a second render on every mount, even though the state initializer already evaluated current conditions moments ago in the same commit.Root Cause
The initializer cleanup (introduced to handle StrictMode double-invocation) detaches all subscriptions. The commit effect then unconditionally re-runs both effects (
ruleEffect.run()+styleEffect.run()), even on fresh mounts where nothing has changed. This doubles the per-mount work: rule matching happens twice, andsetStateforces a second render pass per component.Solution (2 commits)
Commit 1:
hasCommittedRef— distinguish fresh mounts from mid-life replaysPerformance fix: On fresh mounts, cheaply replay the recorded dependency set instead of re-running a full rule pass and forcing a second render.
Effect.dependenciesto record the subscription set before detachingcleanupEffect()now capturesArray.from(effect.observers)before clearinguseNativeCssuses ahasCommittedRefto split the reconnect:observable.get(effect)for each recorded dependency — O(deps) subscription adds, no rule re-evaluation, no forced renderrun()) to catch up on changes that happened while unsubscribedResult: Fresh mounts drop from ~426ms to ~225ms in the benchmark (47% faster), restoring NW4-like performance while preserving all correctness guarantees for StrictMode and Activity lifecycles.
Commit 2:
hasChangedDependencies— handle the hidden pre-render gapCorrectness fix: React 19.2's
<Activity mode="hidden">pre-renders children without mounting effects. Conditions (color scheme, dimensions, container layout) can change in the window between the initializer and the first commit. The cheap replay path must detect this and fall back to the full reconnect.Effect.snapshots: [Observable, unknown][]to capture value-at-detach for each dependencycleanupEffect()now records[observable, observable.get()]pairs before detaching (while the effect is still subscribed, so computed observables return their cached value — cheap reads, not recomputations)hasChangedDependencies(effect)helper: compares current values to snapshots viaObject.is()useNativeCssfresh-mount branch now checkshasChangedDependencies(state.ruleEffect):Scoping eliminates false positives: Per-dependency snapshots mean unrelated observable activity elsewhere in the app (another component's layout, interaction, or theme change) cannot degrade the fresh-mount path. A global change counter would cause false positives; this design only recomputes when this component's rule matching actually read something that changed.
Test Coverage (4 new tests in
reactivity-activity.test.tsx)Performance
Object.is()per dependency; full path only taken when legitimately needed (mid-life or stale conditions)