react-native-windows icon indicating copy to clipboard operation
react-native-windows copied to clipboard

Changing an Image's source prop to an empty object behaves differently than setting its URI property undefined

Open DrewHigginsMSFT opened this issue 1 year ago • 1 comments

Problem Description

When changing an Image component's source prop to be an empty object or one that otherwise does not explicitly define the uri property of that prop's object, the Image component's background ImageBrush never updates to an empty image. However, if you set the source prop to be { uri: undefined }, the ImageBrush does update.

This seems to be due to the former causing the parsed ReactImageSource object here taking a default string value in the former case, but setting it to "null" in the latter case. The former causes this conditional to trigger and return early before modifying the brush.

I patched this in our project by adding a line to set the background brush ad nullptr when that conditional is triggered, but wondering if there should be a larger fix here since the property values of an omitted source vs one explicitly set to undefined theoretically should be the same.

Steps To Reproduce

Declare an Image component and set its source property to be a valid image.

Then, either via a button or timeout, change its source property to be an empty object, and observe that the Image used as the background brush does not change from its old value.

Then. change its source to be { uri: undefined }. Observe that the background brush now updates properly.

Expected Results

Both of these source property values should have the same behavior.

CLI version

7.0.3

Environment

System:
    OS: Windows 10 10.0.22635
    CPU: (16) x64 12th Gen Intel(R) Core(TM) i7-1270P
    Memory: 20.86 GB / 47.67 GB
  Binaries:
    Node: 18.17.1 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 9.6.7 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.18362.0, 10.0.19041.0, 10.0.22621.0
  IDEs:
    Android Studio: Not Found
    Visual Studio: 17.7.34302.85 (Visual Studio Enterprise 2022)
  Languages:
    Java: Not Found
  npmPackages:
    @react-native-community/cli: 7.0.3 => 7.0.3
    react: 18.0.0 => 18.0.0
    react-native: 0.69.3 => 0.69.3
    react-native-windows: 0.69.19 => 0.69.19
  npmGlobalPackages:
    *react-native*: Not Found

Target Platform Version

10.0.19041

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2022

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

const [imageUri, setImageUri] = useState({});
const changeImage = () => setImageUri(imageUri.uri ? {} : { uri: "https://picsum.photos/200/300" });

return (<>
    <Image source={imageUri} style={{ height: 100, width: 100 }} />
    <Button onPress={changeImage} title="Change Image" />
</>);

DrewHigginsMSFT avatar Jan 09 '24 23:01 DrewHigginsMSFT

Version 0.69 is outside of our support window. However I don't think this has been fixed since. Just want to clarify that if a patch were to be made, it wouldn't be backported all the way to 69.

If you have a patch, we'd take a PR though.

Outside of that, we're focused on the Fabric version of Image, working to eventually replace this (and hopefully not replicate the bug). So for us for Paper this would be a Won't Fix. But also we should validate that it does or doesn't repro on Fabric because there is shared code there. @marlenecota can you try this on Fabric?

chrisglein avatar Jan 11 '24 19:01 chrisglein