[RNMobile] Refactor UIManager native module mock #29390
Conversation
|
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
dcalhoun
force-pushed
the
rnmobile/fix/jest-mock-native-uimanager-refactor
branch
3 times, most recently
from
b4751a2 to
fc9ca5c
Compare
|
The following comments are related to the different commits of this PR:
I agree. By the way, I found out that the required function
So initially we could remove the I tried this option but I found complex to mock Additionally, in the current used version of
Yeah, it looks like unnecessary but as per the React navigation documentation related to mocking native modules, they include that mock to silence a warning. I didn't see the mentioned warning but maybe it's because we don't have any test that runs animations. We could remove it but keeping in mind that in the future we might revert this change if we spot the warning.
Totally agree!
Good catch! Let's remove it. |
@fluiddot agreed, we can likely postpone making these changes if you'd rather not attempt to figure them out now.
Ah, good find. I think leaving the mock in place likely makes more sense. Let's just add an inline comment explaining why it's there and potentially include a URL to the relevant documentation. What do you think? Feel free to make whatever changes you would like to this branch/PR or cherry-pick anything onto your own branch. Let me know if you need any help. |
|
I hear this PR will resolve warnings reported in #29653. Can you link this issue in the description so it gets closed when this PR gets merged? 😄 |
The way this native module was being mocked didn't work because UIManager is not exposed in NativeModules object and doMock wasn't overriding it properly.
We currently reference TextStateInput (a private module) within react-native-aztec/src/AztecView. Doing so requires that we mock it via its internal path to avoid "TypeError: Cannot read property 'Commands' of undefined." The private module referenced could possibly be replaced with a React ref instead. We could then remove this internal mock.
Generally, named exports are used when importing React Native modules. This changes the one exception in our source were we imported a default import of `ReactNative`. Importing this default also caused issues when attempting to mock `react-native` for Jest. This change will make future mocking in Jest easier.
The `providesModuleNodeModules` option was removed from Jest. Including it in the Jest configuration displayed a warning on each run. https://git.io/JtNRI > Unknown option "haste.providesModuleNodeModules" with value ["react-native", "react-native-svg"] was found. This is probably a typing mistake. Fixing it will remove this message.
Explain why we are mocking this internal React Native module for future reference.
dcalhoun
changed the base branch from
rnmobile/fix/jest-mock-native-uimanager
to
trunk
dcalhoun
force-pushed
the
rnmobile/fix/jest-mock-native-uimanager-refactor
branch
from
fc9ca5c to
3abfccd
Compare
Sure! I'll review it today and if it's ready I'll merge it into Thank you very much for finish the work on this PR, I had it in my TODO list but I couldn't tackle it sooner 🙇 . |
fluiddot
added
the
Mobile App - i.e. Android or iOS
There was a problem hiding this comment.
LGTM 🎊 !
I tested the following:
- Run verse unit test (
npm run native test verse) withisSelectedprop inRichText, the test passed 🟢 - Run all unit tests (
npm run native test), the tests passed 🟢 - Verify that the
blurandfocusbehaviours work as expected, tested on iPhone 8 simulator and Pixel 4 emulator 🟢
fluiddot
changed the title
Description
Further exploration of #29375.
How has this been tested?
None of the current unit tests are calling the focus or blur on native components so for testing this, an extra modification is required.
The following steps will force the
RichTextcomponent of theVerseblock to be selected, this will make theRichTextcomponent to call the focus function on theAztecViewnative component when it's mounted:isSelectedprop toRichTextcomponent inverse/edit.js.npm run native test verse.Screenshots
n/a
Types of changes
Bug fix
Checklist: