fix(android): initial measuring tweaks - #784
Open
hejsztynx wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an Android-only race in initial height estimation for EnrichedTextInputView when defaultValue is set and fontSize is missing/invalid, by preventing MeasurementStore from caching “fresh” sizes until the shadow node can actually be invalidated.
Changes:
- Default
fontSizeused duringinitialMeasurenow falls back to the view’s default whenfontSizeisnullor<= 0. MeasurementStore.storenow invalidates the shadow node via a callback and only commits the new cached size when invalidation succeeds.- Initializes
forceHeightRecalculationCounter_in the shadow node and aligns the counter value sent via state updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| android/src/main/new_arch/react/renderer/components/ReactNativeEnrichedSpec/EnrichedTextInputShadowNode.h | Initializes forceHeightRecalculationCounter_ to avoid undefined comparisons. |
| android/src/main/java/com/swmansion/enriched/textinput/MeasurementStore.kt | Avoids caching new measured sizes unless shadow-node invalidation succeeds; improves fontSize fallback in initialMeasure. |
| android/src/main/java/com/swmansion/enriched/textinput/EnrichedTextInputViewLayoutManager.kt | Passes an invalidation callback into MeasurementStore.store and updates the state counter post-increment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Summary
When
fontSizewasn't provided, the initial component height calculation was sometimes wrong ifdefaultValuewas set.During the calculation in
initialMeasure, iffontSizewasn't provided via props, its value was 0, which caused the initial height estimation to be completely off. This edge case allowed to expose the real issue:Normally, the layout should be refreshed even if the
initialMeasurewas off, but the calculated values were incorrectly cached, which didn't allow that. After runinitialMeasurethe shadow node received the estimated (wrong) height. After that thesetValueruns, as we provideddefaultValue, which requests a layout recalculation. The recalculation is done correctly, but thestateWrapperis not yet initialized (isnull), which stops us from telling the shadow node to recalculate the layout, but still, the calculated sizes inMeasurementStoreare cached anyway. That wayMeasurementStorewill not try to ask the shadow node for layout invalidation, as it incorrectly cached that the layout is fresh. This is now fixed and we only cache the sizes only ifstateWrapperis present and we can safely assume that the information was delivered to the shadow node.In
initialMeasure, thefontSizenow fallbacks to the default font size if<= 0. That's how it was already done in the initialEnrichedTextmeasurements.I've also tweaked two small things: in
EnrichedTextInputShadowNode.cpptheforceHeightRecalculationCounter_was never initialized before comparing its value, which would make it have random, garbage values. I've also adjusted the providedforceHeightRecalculationCounterto thestateWrapper, as its value was always pre-increment. I believe it's better to have them consistent.Test Plan
I've provided a modified example app, so you can see the bug yourself - you can run
and then you can check out how it works after the fix
Screenshots / Videos
The race condition in action (stumbled across in a different app):
Screen.Recording.2026-08-27.at.12.12.20.mov
Compatibility
Checklist