Refactor: ContactCardModal to use func comp and useQuery #661
Conversation
JF-Cozy
force-pushed
the
refactor/contactCardModal
branch
from
0133658 to
18caa13
Compare
Can you explain what was wrong with the latest react-redux? Is there any existing issue on Enzyme / Redux repo? |
JF-Cozy
force-pushed
the
refactor/contactCardModal
branch
3 times, most recently
from
c8eabc3 to
21e3eae
Compare
|
|
||
| // Uses a different version of react-redux | ||
| // to prevent Enzyme's incompatibility with actual react-redux version | ||
| jest.mock('react-redux', () => require('react-redux-test')) |
There was a problem hiding this comment.
Can you link to an issue or something?
There was a problem hiding this comment.
What if you use mount instead of shallow?
I really don't like having 2 versions of react-redux, the test doesn't test the code used in our app...
There was a problem hiding this comment.
This hack is not global, only on this file. Me neither but I didn't success to get it work. With mount we need to wrap the function's calls like realConnectedComponent.prop('toggleSelection')({ _id: 1, id: 1 }) in act(). But even doing this, the call does nothing, neither the .update().
There was a problem hiding this comment.
Even if the hack is scoped, the test is not relevant anymore. We should create an issue on Contacts to rewrite this test with react-testing-lib. Since one of the next task will be to work on list's filter, maybe we will have to work on the selection part and fix this issue
|
|
||
| const contact = | ||
| resultContactById.data !== null ? resultContactById.data[0] : {} | ||
| const allGroups = resultAllGroups.data !== null ? resultAllGroups.data : [] |
There was a problem hiding this comment.
not sure we need this resultAllGroups.data !== null anymore since we use allGroups only when showResult is true === data loaded, no ? Same for resultContactById ?
There was a problem hiding this comment.
We can simplify by using showResult but at the first render the data are still null. But we can remove those const and use the data directly in the return part (wrap into showResult)
There was a problem hiding this comment.
You right, the data will be null at the first render. But we don't use them at this time, no? We only read them if showResult (or dataHaveBeenLoaded) is true, right? So we don't need to deal with null data, right?
There was a problem hiding this comment.
We only read them if showResult (or dataHaveBeenLoaded) is true, right?
Yes. Code updated according to use the data directly in the return part. What do you think ?
There was a problem hiding this comment.
Better! But I'm not very convinced by reading resultContactById.data[0]. Are you sure that getById() returns an array and not the object? 🤔
There was a problem hiding this comment.
Yes... It will not work otherwise... Besides, browser returns this
Failed prop type: Invalid prop `contact` of type `array` supplied to `ContactCard`, expected `object`.
There was a problem hiding this comment.
I think that Q().getById() should returns the related document, not an array containing one document. It should behave like the DocumentCollection.get(), no? If yes, we should open an issue on cozy-client
There was a problem hiding this comment.
issue : cozy/cozy-client#764
JF-Cozy
force-pushed
the
refactor/contactCardModal
branch
from
21e3eae to
693395e
Compare
chore: Update react-redux and add Provider
JF-Cozy
force-pushed
the
refactor/contactCardModal
branch
from
693395e to
f1a892b
Compare
Yes, first message updated |
This PR is a part [1] of multiple PR about the new sort logic in the application.
This refactor prevents problems of using
useQueryinApp. Otherwise It triggers errors when usinguseQueryinAppandQueryhere inContactCardModal.react-redux> 7.1.0 to be able to useuseQueryhookcozy-clientand fix some testsreact-redux-test(react-reduxnot updated) to prevent broken tests andEnzymeincompatibilityContactCardModalin functional component usinguseQuery(*) Using older version of
react-reduxis the best solution I found, I didn't success to make tests working in other way (like usingmountofEnzymeorreact-test-renderer, or without Provider). The problem is that.dive()doesn't work anymore and even if the right component is selected, the.update()doesn't do anythingSee enzymejs/enzyme#2202 , enzymejs/enzyme#2302