Feature: add warnings #32
Conversation
JuraJuki
commented
Aug 20, 2020
JuraJuki
commented
- adds warnings to usePost, usePatch
- references issue: Validate if hooks show warnings on passing wrong JSONA models to data parameter #27
There was a problem hiding this comment.
Well done! This seems actually really cool as it could benefit users who don't use TypeScript.
Only check small improvements regarding validator function.
Although this will be very beneficial, I believe issue #43 is still unresolved with this PR. This can be confirmed with the current state of usePost test ->
aardvark/src/tests/postHookTests.test.tsx
Line 54 in cbc6dbc
We'll probably need to do some additional digging to fix that, but this is a nice improvement nevertheless.
src/hooks/usePatch.ts
Outdated
| @@ -63,6 +65,23 @@ export const usePatch = < | |||
| ) as unknown) as F; | |||
| }); | |||
|
|
|||
| const checkValidity = ( | |||
There was a problem hiding this comment.
This function can actually live outside the hook because it's pure and does not rely on some scoped variable. If extracted outside the hook, it would be a little bit lighter to process because the function does not need to be initialized every time the hook is called.
src/hooks/usePost.ts
Outdated
| @@ -52,6 +54,18 @@ export const usePost = < | |||
| ) as unknown) as F; | |||
| }); | |||
|
|
|||
| const checkValidity = (serializeModelParam: ActionPostData) => { | |||
# Conflicts: # src/hooks/usePost.ts
|
@renatoruk extracted, fixed conflicts. What is left to be done is that the model typings issue #43 is resolved |
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
- Coverage 85.41% 85.33% -0.08%
==========================================
Files 49 49
Lines 1049 1064 +15
Branches 161 168 +7
==========================================
+ Hits 896 908 +12
- Misses 151 154 +3
Partials 2 2
Continue to review full report at Codecov.
|
@JuraJuki I added codecov action to the CI pipeline. Now, after merging master into this branch we got a drop in code coverage. You can see the detailed report here (look for the red squares). I suggest we increase the coverage for this particular feature and then we merge? |
