Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image.resolveAssetSource(require(path/${image}.png')) does not work on tests as previous version #41907

Closed
idrissakhi opened this issue Dec 12, 2023 · 8 comments
Labels
Component: Image Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Needs: Triage 🔍 Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@idrissakhi
Copy link

idrissakhi commented Dec 12, 2023

Description

Describe the bug

const emailMailImage = require('assets/images/email-mail/mail.png');

const emailMailImageDimensions = Image.resolveAssetSource(emailMailImage);

TypeError: Cannot read properties of undefined (reading 'width')Jest

Expected behavior
Tests passe on 0.72.X

Steps to Reproduce
install react native 0.73.0 and run a test having

        source={emailMailImage}
        resizeMode={'contain'}
        style={[
          styles.emailSentImage,
          {
            width: resizeEmailImageWidth,
            aspectRatio: emailMailImageDimensions.width / emailMailImageDimensions.height,
          },
        ]}
      />
      ```

Versions

npmPackages:
@testing-library/react-native: 12.4.1 => 12.4.1
react: 18.2.0 => 18.2.0
react-native: 0.73.0 => 0.73.0
react-test-renderer: 18.2.0 => 18.2.0

Steps to reproduce

Steps to Reproduce
install react native 0.73.0 and run a test having

        source={emailMailImage}
        resizeMode={'contain'}
        style={[
          styles.emailSentImage,
          {
            width: resizeEmailImageWidth,
            aspectRatio: emailMailImageDimensions.width / emailMailImageDimensions.height,
          },
        ]}
      />

React Native Version

0.73.0

Affected Platforms

Runtime - iOS

Output of npx react-native info

System:
  OS: macOS 14.1.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 906.72 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.12.0
    path: ~/.nvm/versions/node/v18.12.0/bin/node
  Yarn:
    version: 1.22.19
    path: /usr/local/bin/yarn
  npm:
    version: 8.19.2
    path: ~/.nvm/versions/node/v18.12.0/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.14.3
    path: /Users/sakhiidris/.rvm/gems/ruby-3.0.0/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.0
      - iOS 17.0
      - macOS 14.0
      - tvOS 17.0
      - watchOS 10.0
  Android SDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.10811636
  Xcode:
    version: 15.0.1/15A507
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 3.0.0
    path: /Users/sakhiidris/.rvm/rubies/ruby-3.0.0/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.0
    wanted: 0.73.0
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Stacktrace or Logs

TypeError: Cannot read properties of undefined (reading 'width')Jest

Reproducer

c

Screenshots and Videos

No response

Copy link

⚠️ Missing Reproducible Example
ℹ️ We could not detect a reproducible example in your issue report. Please provide either:
  • If your bug is UI related: a Snack
  • If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

@github-actions github-actions bot added Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Component: Image labels Dec 12, 2023
@cortinico
Copy link
Contributor

@idrissakhi could you add a repro?

@delphinebugner
Copy link
Contributor

delphinebugner commented Dec 15, 2023

I have the same using react-native-fast-image, which is also using resolveAssetSource to get the image sources. My snapshots break with empty URI for all images.

It's because in 0.73 this function resolveAssetSource is mocked with empty return :

// in packages/react-native/jest/setup.js
  .mock('../Libraries/Image/Image', () => {
    const Image = mockComponent('../Libraries/Image/Image');
    Image.getSize = jest.fn();
    Image.getSizeWithHeaders = jest.fn();
    Image.prefetch = jest.fn();
    Image.prefetchWithMetadata = jest.fn();
    Image.queryCache = jest.fn();
    Image.resolveAssetSource = jest.fn(); ⬅️⬅️⬅️ 🐞🐞🐞

    return Image;
  })

A quick fix could be:

 Image.resolveAssetSource = jest.fn().mockImplementation(source => source);

New image mock was introduced in 0.73, before we only had :
.mock('../Libraries/Image/Image', () => mockComponent('../Libraries/Image/Image'))

delphinebugner pushed a commit to delphinebugner/react-native that referenced this issue Dec 15, 2023
resolveAssetSource should not return nothing by default as some librairies (ex: react native fast image) rely on it to compute the image source
without this mock implementation, all sources disappear from images in test environnement
see issue facebook#41907

 the input source - leaving it empty breaks
Copy link

github-actions bot commented Jan 9, 2024

This issue is waiting for author's feedback since 24 days. Please provide the requested feedback or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 9, 2024
Copy link

This issue was closed because the author hasn't provided the requested feedback after 7 days.

facebook-github-bot pushed a commit that referenced this issue Mar 18, 2024
Summary:
### Context

- Since RN 0.73, in the jest.setup file, methods of the Image module (Image.getSize, Image.resolveAssetSource...) are mocked on the **JS side** (introduced in #36996)
- It causes issues like #41907 : `Image.resolveAssetSource` returns nothing in test env with the new JS mock, when some test relies on it.
- On my project, it broke the snapshots : the URL of images disappeared. I use `react-native-fast-image` which uses `Image.resolveAssetSource` to compute URLs.
- I first opened a PR to fix exclusively Image.resolveAssetSource: #41957. I will close it to focus on this new one.
- As suggested by ryancat and idrissakhi, it should be better to return to the previous mock, where no method is mocked on the JS side, and we can trust the actual JS to work in test.

This is what this PR intends to do.

### Content

Along fixing the Image module mock in jest.setup, this PR :

- adds unit test on each one of the methods, ensuring they have a consistent behavior even when the module is mocked.
- adds 3 missing native mocks for `NativeImageLoader`: `prefetchImageWithMetadata`, `getSizeWithHeaders` & `queryCache`. After this PR, no method from NativeImageLoader remains unmocked.

## Changelog:

[GENERAL][FIXED] - fix jest setup for Image methods (resolveAssetSource, getSize, prefetch, queryCache)

Pull Request resolved: #43497

Test Plan:
See exhaustive unit tests in PR.

You can re-use the mock with all the methods mocked and see how the new unit tests fail.

I also patched those changes on my project: my snapshot did have their URL back (see demonstrative screenshots in my original PR: #41957 - NB; fixed mock was different but result was the same -> those screenshots cover only two cases, but anyway they illustrate well the case!)

Reviewed By: ryancat

Differential Revision: D54959063

Pulled By: tdn120

fbshipit-source-id: 837266bd6991eb8292d9f6af1774e897ac7a8890
@ducminhleeh
Copy link

Do we have a plan to delegate this fix to 0.73.x, I still got a same error

@JacobDel
Copy link

seems still present in 0.74.3 and 0.74.5. Only 0.74.4 works for me. What is going on?

@delphinebugner
Copy link
Contributor

@JacobDel yes, the fix will be present in the 0.75 only (see d53cc2b)

You can use this patch like - cc @ducminhleeh:

diff --git a/node_modules/react-native/jest/setup.js b/node_modules/react-native/jest/setup.js
index e98550f..a829f99 100644
--- a/node_modules/react-native/jest/setup.js
+++ b/node_modules/react-native/jest/setup.js
@@ -109,17 +109,8 @@ jest
       Constants: {},
     },
   }))
-  .mock('../Libraries/Image/Image', () => {
-    const Image = mockComponent('../Libraries/Image/Image');
-    Image.getSize = jest.fn();
-    Image.getSizeWithHeaders = jest.fn();
-    Image.prefetch = jest.fn();
-    Image.prefetchWithMetadata = jest.fn();
-    Image.queryCache = jest.fn();
-    Image.resolveAssetSource = jest.fn();
-
-    return Image;
-  })
+  .mock('../Libraries/Image/Image', () => mockComponent('../Libraries/Image/Image'),
+  )
   .mock('../Libraries/Text/Text', () =>
     mockComponent('../Libraries/Text/Text', MockNativeMethods),
   )

It's the one I use on 0.73.7; maybe there is some line number difference in 0.74.X, I hav'nt checked. It should not be hard to adapt, the idea is to replace the Image mock by the default one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Image Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Needs: Triage 🔍 Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

6 participants
@cortinico @JacobDel @delphinebugner @idrissakhi @ducminhleeh and others