bugsnag / bugsnag-expo

MIT License
11 stars 5 forks source link

Expo constants are undefined when executing tests #96

Open sayo96 opened 1 year ago

sayo96 commented 1 year ago

Describe the bug

A clear and concise description of what the bug is. I'm running into an error which says "Can't read manifest of undefined" when running tests. I do have a mock in place but i guess the issue lies within bugsnag-expo

For some weird reason, Constants appear as undefined here https://github.com/bugsnag/bugsnag-expo/blob/21899845975c3857b7b398d4878e04316ee5b72b/packages/expo/src/config.js#L12

Please do note that i have a mock function which mocks the expo constants as well

Steps to reproduce

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Environment

Example Repo

Example code snippet

# (Insert code sample to reproduce the problem)
Error messages: ● Test suite failed to run TypeError: Cannot read property 'manifest' of undefined at Object. (node_modules/@bugsnag/expo/src/config.js:12:15) at Object. (node_modules/@bugsnag/expo/src/notifier.js:17:64) ``` ```

This might be the possible fix : https://github.com/expo/eas-cli/issues/1265#issuecomment-1301525320

If yes, please implement it and publish it in a next release so i could use it

johnkiely1 commented 1 year ago

Hi @sayo96,

I'm not sure the suggested fix is the same issue, as it appears to be catering for the scenario where Constants.manifest returns null. For some modern expo apps we would expect Constants.manifest to return null in which case we check Constants.manifest2. In your case it's Constants that is returning undefined so never checks for either manifest.

At what point are you seeing this error?

Would you mind sharing some details on how you are mocking the various packages? Especially the expo-constants one.

Thanks

sayo96 commented 1 year ago

Hey @johnkiely1 . I apologise for the delayed reply. I was caught up with other expo bugs.

Here is what we did :

I had to add stuff in default to prevent the manifest bug which is kinda hack tbh.. Can you also let me know why the mock file was not considered?

johnkiely1 commented 1 year ago

Thanks @sayo96,

I'm glad you've found a fix. It looks like any code uses require then default needs to be defined as that’s how expo-constants is exported, so what you've done is actually the correct way to resolve this and not so much of a hack.

Having said that we are looking into the possibly at replacing our usage of require with import which would be another way for this to be resolved. This has been added to the backlog, I don't have an ETA for this but we will post updates here as soon as we get them.

sayo96 commented 1 year ago

@johnkiely1 Sounds good thanks a ton :)