diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.kt index 3f413faaf7c0..b894ac8129f6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.kt @@ -369,8 +369,12 @@ public class ReactImageView( // no worth marking as dirty if it already rendered. } + /** Sets the request headers. A change rebuilds the image request on the next update. */ public fun setHeaders(headers: ReadableMap?) { - this.headers = headers + if (this.headers != headers) { + this.headers = headers + isDirty = true + } } // Disable rasterizing to offscreen layer in order to preserve background effects like box-shadow diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/image/ReactImagePropertyTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/image/ReactImagePropertyTest.kt index c3b78c3f3834..b69f015595e0 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/image/ReactImagePropertyTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/image/ReactImagePropertyTest.kt @@ -15,7 +15,9 @@ import android.graphics.Color import android.util.DisplayMetrics import com.facebook.common.logging.FLog import com.facebook.drawee.backends.pipeline.Fresco +import com.facebook.drawee.backends.pipeline.PipelineDraweeControllerBuilder import com.facebook.drawee.drawable.ScalingUtils +import com.facebook.imagepipeline.request.ImageRequest import com.facebook.react.bridge.Arguments import com.facebook.react.bridge.BridgeReactContext import com.facebook.react.bridge.CatalystInstance @@ -23,6 +25,7 @@ import com.facebook.react.bridge.JavaOnlyMap import com.facebook.react.bridge.ReactTestHelper.createMockCatalystInstance import com.facebook.react.common.ReactConstants import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests +import com.facebook.react.modules.fresco.ReactNetworkImageRequest import com.facebook.react.uimanager.BackgroundStyleApplicator import com.facebook.react.uimanager.DisplayMetricsHolder import com.facebook.react.uimanager.LengthPercentage @@ -40,12 +43,15 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.MockedStatic +import org.mockito.Mockito.RETURNS_SELF import org.mockito.Mockito.anyString import org.mockito.Mockito.mockStatic import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.reset +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.robolectric.RobolectricTestRunner import org.robolectric.RuntimeEnvironment @@ -281,4 +287,27 @@ class ReactImagePropertyTest { viewManager.setHeaders(mockView, headers) verify(mockView).setHeaders(headers) } + + @Test + fun testHeadersChangeRebuildsImageRequest() { + val controllerBuilder = mock(defaultAnswer = RETURNS_SELF) + val viewManager = ReactImageManager(controllerBuilder) + val view = viewManager.createViewInstance(themeContext) + val sources = Arguments.createArray() + sources.pushMap(JavaOnlyMap.of("uri", "https://example.com/image.png")) + viewManager.setSource(view, sources) + viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-1")) + view.maybeUpdateView() + + viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-2")) + view.maybeUpdateView() + viewManager.setHeaders(view, JavaOnlyMap.of("Authorization", "token-2")) + view.maybeUpdateView() + + val requests = argumentCaptor() + verify(controllerBuilder, times(2)).setImageRequest(requests.capture()) + assertThat(requests.firstValue.sourceUri).isEqualTo(requests.secondValue.sourceUri) + assertThat((requests.secondValue as ReactNetworkImageRequest).headers) + .isEqualTo(JavaOnlyMap.of("Authorization", "token-2")) + } } diff --git a/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp index 8cb669ebf127..2785fa720679 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/image/ImageProps.cpp @@ -12,6 +12,9 @@ #include #include +#include +#include + namespace facebook::react { ImageProps::ImageProps( @@ -145,6 +148,30 @@ static folly::dynamic convertEdgeInsets(const EdgeInsets& edgeInsets) { return edgeInsetsResult; } +// ImageSource::operator== compares only the request identity: type and URI. +static bool hasSamePayload(const ImageSource& lhs, const ImageSource& rhs) { + return std::tie( + lhs.type, + lhs.uri, + lhs.bundle, + lhs.scale, + lhs.size, + lhs.body, + lhs.method, + lhs.cache, + lhs.headers) == + std::tie( + rhs.type, + rhs.uri, + rhs.bundle, + rhs.scale, + rhs.size, + rhs.body, + rhs.method, + rhs.cache, + rhs.headers); +} + ComponentName ImageProps::getDiffPropsImplementationTarget() const { return "Image"; } @@ -158,11 +185,14 @@ folly::dynamic ImageProps::getDiffProps(const Props* prevProps) const { folly::dynamic result = ViewProps::getDiffProps(oldProps); - if (sources != oldProps->sources) { + if (!std::ranges::equal(sources, oldProps->sources, hasSamePayload)) { auto sourcesArray = folly::dynamic::array(); for (const auto& source : sources) { sourcesArray.push_back(toDynamic(source)); } + if (!sources.empty()) { + result["headers"] = sourcesArray[0]["headers"]; + } result["source"] = sourcesArray; } @@ -221,7 +251,8 @@ folly::dynamic ImageProps::getDiffProps(const Props* prevProps) const { SharedColor prevTintColorValue = oldProps->tintColor.value_or(SharedColor{}); if (tintColorValue != prevTintColorValue) { - result["tintColor"] = *tintColorValue; + result["tintColor"] = tintColorValue ? folly::dynamic(*tintColorValue) + : folly::dynamic(nullptr); } } diff --git a/packages/react-native/ReactCommon/react/renderer/components/image/tests/ImagePropsDiffTest.cpp b/packages/react-native/ReactCommon/react/renderer/components/image/tests/ImagePropsDiffTest.cpp new file mode 100644 index 000000000000..22720793e0b3 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/image/tests/ImagePropsDiffTest.cpp @@ -0,0 +1,93 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include +#include + +#ifdef RN_SERIALIZABLE_STATE + +namespace facebook::react { + +namespace { + +using folly::dynamic; + +// Props 2.0 serializes every field of a source, JS only the ones it sets. +void expectPayloads(const std::vector& commits) { + expectPayloadsMatchRawProps(commits, {"source"}); +} + +} // namespace + +// ReactImageView.setSource reads width and height at the top level of each +// source when there are several, and ReactImageManager reads the request +// headers from the top-level headers prop, which JS derives from the first +// source. +TEST(ImagePropsDiffTest, multipleSourcesCarrySizesAndHeaders) { + dynamic headers = dynamic::object("Authorization", "token"); + expectPayloads({{dynamic::object( + "source", + dynamic::array( + dynamic::object("uri", "https://a")("width", 10)("height", 20)( + "headers", headers), + dynamic::object("uri", "https://b")("width", 30)("height", 40)))( + "headers", headers)}}); +} + +// ReactImageManager ignores a null headers prop, so only an empty map clears +// the headers of the previous source. +TEST(ImagePropsDiffTest, clearsHeadersWhenTheNewSourceHasNone) { + dynamic headers = dynamic::object("Authorization", "old"); + expectPayloads({ + {dynamic::object( + "source", + dynamic::array( + dynamic::object("uri", "https://a")("headers", headers)))( + "headers", headers)}, + {dynamic::object( + "source", dynamic::array(dynamic::object("uri", "https://b")))( + "headers", nullptr), + dynamic::object("headers", dynamic::object())}, + }); +} + +// ReactImageManager clears the tint only for null; 0 is a transparent SRC_IN +// tint that hides the image. +TEST(ImagePropsDiffTest, removingTintColorEmitsNull) { + expectPayloads({ + {dynamic::object("tintColor", static_cast(0xFFFF0000))}, + {dynamic::object("tintColor", nullptr)}, + }); +} + +// ReactImageView reads the cache policy and the sizes of each source, and +// ReactImageManager the headers; a source with the same URI can change them. +TEST(ImagePropsDiffTest, emitsASameUriSourceWhenItsOtherFieldsChange) { + dynamic headers = dynamic::object("Authorization", "old"); + dynamic source = dynamic::object("uri", "https://a")("width", 10)( + "height", 20)("scale", 1)("cache", "default")("headers", headers); + std::vector commits{ + {dynamic::object("source", dynamic::array(source))("headers", headers)}}; + dynamic changes = dynamic::object("cache", "reload")("width", 30)("scale", 2); + for (const auto& [field, value] : changes.items()) { + source[field] = value; + // Props 2.0 sends the first source's headers whenever the sources change. + commits.push_back( + {dynamic::object("source", dynamic::array(source)), + dynamic::object("headers", headers)}); + } + source["headers"] = dynamic::object("Authorization", "new"); + commits.push_back({dynamic::object("source", dynamic::array(source))( + "headers", source["headers"])}); + expectPayloads(commits); +} + +} // namespace facebook::react + +#endif diff --git a/packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h b/packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h index 86756373d6da..17593ca52f78 100644 --- a/packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h +++ b/packages/react-native/ReactCommon/react/renderer/imagemanager/primitives.h @@ -62,10 +62,8 @@ class ImageSource { imageSourceResult["bundle"] = bundle; imageSourceResult["scale"] = scale; - folly::dynamic sizeResult = folly::dynamic::object(); - sizeResult["width"] = size.width; - sizeResult["height"] = size.height; - imageSourceResult["size"] = sizeResult; + imageSourceResult["width"] = size.width; + imageSourceResult["height"] = size.height; imageSourceResult["body"] = body; imageSourceResult["method"] = method; diff --git a/packages/react-native/ReactCommon/react/test_utils/PropsPayloads.h b/packages/react-native/ReactCommon/react/test_utils/PropsPayloads.h new file mode 100644 index 000000000000..88da44225401 --- /dev/null +++ b/packages/react-native/ReactCommon/react/test_utils/PropsPayloads.h @@ -0,0 +1,131 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#ifdef RN_SERIALIZABLE_STATE + +namespace facebook::react { + +/* + * One commit: the props JS sends (a removed prop is null), and the Props 2.0 + * values that intentionally differ from what Props 1.5 sends. + */ +struct PropsCommit { + folly::dynamic props; + folly::dynamic intendedDifferences = folly::dynamic::object(); +}; + +namespace detail { + +inline bool payloadValuesMatch(const folly::dynamic &actual, const folly::dynamic &expected) +{ + if (actual.isNumber() && expected.isNumber()) { + // Props 2.0 stores floats, so 0.998 comes back as 0.99800002574... + auto tolerance = 1e-6 * std::max(1.0, std::abs(expected.asDouble())); + return std::abs(actual.asDouble() - expected.asDouble()) <= tolerance; + } + if (actual.isArray() && expected.isArray()) { + return actual.size() == expected.size() && + std::equal(actual.begin(), actual.end(), expected.begin(), payloadValuesMatch); + } + if (actual.isObject() && expected.isObject()) { + return actual.size() == expected.size() && std::ranges::all_of(expected.items(), [&](const auto &item) { + return actual.count(item.first) != 0 && payloadValuesMatch(actual[item.first], item.second); + }); + } + return actual == expected; +} + +// Each element carries the fields of the expected element, and may add more. +inline bool elementsContainFields(const folly::dynamic &actual, const folly::dynamic &expected) +{ + if (!actual.isArray() || !expected.isArray() || actual.size() != expected.size()) { + return payloadValuesMatch(actual, expected); + } + for (size_t i = 0; i < expected.size(); i++) { + for (const auto &[field, value] : expected[i].items()) { + if (actual[i].count(field) == 0 || !payloadValuesMatch(actual[i][field], value)) { + return false; + } + } + } + return true; +} + +} // namespace detail + +/* + * Applies the commits in order and checks that each Props 2.0 payload + * (`getDiffProps`) has the keys, nulls and values of the Props 1.5 payload + * (accumulated raw props), apart from the commit's intended differences. The + * elements of the arrays under `keysWithAddedFields` may carry fields that JS + * does not send. + */ +template +void expectPayloadsMatchRawProps( + const std::vector &commits, + const std::set &keysWithAddedFields = {}) +{ + ContextContainer contextContainer{}; + PropsParserContext parserContext{-1, contextContainer}; + + auto previous = std::make_shared(); + folly::dynamic previousRawProps = folly::dynamic::object(); + for (size_t i = 0; i < commits.size(); i++) { + const auto &commit = commits[i]; + auto rawProps = RawProps(commit.props); + auto parser = RawPropsParser(); + parser.template prepare(); + rawProps.parse(parser); + auto props = std::make_shared(parserContext, *previous, rawProps); + auto accumulatedRawProps = mergeDynamicProps(previousRawProps, commit.props, NullValueStrategy::Override); + + auto actual = props->getDiffProps(i == 0 ? nullptr : previous.get()); + auto expected = i == 0 ? accumulatedRawProps : diffDynamicProps(previousRawProps, accumulatedRawProps); + for (const auto &[key, value] : commit.intendedDifferences.items()) { + expected[key] = value; + } + + std::set keys; + for (const auto &payload : {actual, expected}) { + for (const auto &key : payload.keys()) { + keys.insert(key.asString()); + } + } + for (const auto &key : keys) { + bool matches = actual.count(key) != 0 && expected.count(key) != 0 && + (keysWithAddedFields.contains(key) ? detail::elementsContainFields(actual[key], expected[key]) + : detail::payloadValuesMatch(actual[key], expected[key])); + EXPECT_TRUE(matches) << "commit " << i << " " << commit.props << ", key '" << key + << "'\n Props 2.0: " << actual.getDefault(key, "(absent)") + << "\n expected: " << expected.getDefault(key, "(absent)"); + } + + previous = props; + previousRawProps = accumulatedRawProps; + } +} + +} // namespace facebook::react + +#endif