Repository navigation
fix(native): map native styles onto new objects, leaving the computed props untouched - #505
Closed
YevheniiKotyrlo wants to merge 1 commit into
Closed
YevheniiKotyrlo wants to merge 1 commit into
YevheniiKotyrlo wants to merge 1 commit into
Conversation
… props untouched nativeStyleMapping drained the computed props in place. Those props are cached and shared by every render, and an Image with object-fit maps a copy whose nested objects are still the cached ones, so a mapped value was lost when another config followed it. Each config now maps the props the config before it produced, and the mapping copies only the objects it changes.
Contributor
Author
|
Folded into #435, which needs this change to stay correct. |
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.
Problem
nativeStyleMappingdeletes each mapped declaration from its source and writes the value into the props it is handed. Those props are the computed props cached for every render of the component, andgetStyledPropshands them over once per config throughadaptProps. For most components that is the cache itself, so a config sees an earlier config's mapping only because the cache was changed. For React Native'sImagewithobject-fit,adaptImagePropsreturns a new object on each call, so a config that follows a mapping starts from a fresh copy and the mapped value is lost:Solution
nativeStyleMappingbuilds new objects along the paths it changes and leaves its input as it was, withwithDeepPathandwithoutKeysbesidegetDeepPathandsetDeepPathinsrc/native/objects.ts.getStyledPropsadapts the computed props once per render, and each config maps the props the config before it produced.Tests
src/__tests__/native/image-fit.test.tsx: on anImagewithobject-fit, a tint mapping followed by another config keepstintColoron the first render and after a re-render, for a normal and an!importantdeclaration. Onmainthe normal case loses it on the first render.src/__tests__/native/styled.test.ios.tsx: a mapping onto a nested target maps nothing before its class is set, then the same value on every render.src/__tests__/native/objects.test.ts:withDeepPathandwithoutKeyscopy what they change and leave their input untouched.Verification
Every
ci.ymljob passes on this head with #477's harness fix: lint, typecheck, the tests, both builds, the web export, nothing unstaged, commitlint, and the iOS dev build on macOS 27.0.1 with Xcode 27.0, where CI pins 26.6.A render costs the same: 3000 re-renders of a mapped
Viewtake 61.3 µs each here against 62.3 µs onmain, a plainView16.7 µs against 16.3, and a mappedImagewithobject-fit27.0 µs against 29.0, each the median of five runs on Windows with Node 26, within each side's spread.No existing issue tracks this. I searched for
nativeStyleMapping,tintColor,object-fitandnativeStyleToProp.Merge order
target: falseconfig's declarations into a bucket of its own, which this keeps intact across renders; it will be stacked on this.