fix: Use .mock extension for MMKV mocks
#647
Open
Conversation
This file contains 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
added 2 commits
March 11, 2024 23:50
.mock files are supposed to replace their regular counterparts when running in mock mode. It doesn't make much sense to import from both and have a check to determine which to use. Instead, metro should pick the one that is currently relevant. This is achieved by giving the functions the same name and only importing from the regular one. the .mock one is used when running in mock mode.
jest doesn't pick the .mock file by default. adding jest.mock in the test file solves this. If there are more testFiles, the mock can be made globally in a jest.setup
mrousavy
changed the title
.mock extension for MMKV mocks
|
Thanks for your PR, this looks good to me! I'll do some tests to confirm. The .mock files will be compiled out I assume? |
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.
The issue I was having that when I was running metro in mock mode. isJest() returned false, so the createMMKV from the regular file was imported, but since I was running in mock mode. That file didn't exist, so createMMKV was undefined.
Maybe I am missing something, but .mock files are supposed to replace their regular counterparts when running in mock mode. It doesn't make much sense to me to import from both and have a check to determine which to use. Instead, metro should pick the one that is currently relevant. This is achieved by giving the functions the same name and only importing from the regular one. the .mock one is used when running in mock mode.
For jest to use the mock I have added jest.mock in the unit test. This feels like a more appropriate way to add a mock then changing the imports in the actual code.
Alternatively, we could add a .mock for that platform checker that always returns true, that is how I initially patched it and a smaller change.
I am not sure what the .web variant is for, I didn't want to remove it, since it is unrelated, but it does not appear to be used anywhere.
This is my first PR to this repo, please let me know if I should do something else or in a different way.