feat: new hook useGetSet #530
Conversation
Codecov Report
@@ Coverage Diff @@
## master #530 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 45 +1
Lines 776 788 +12
Branches 138 138
=========================================
+ Hits 776 788 +12
Continue to review full report at Codecov.
|
src/useGetSet/useGetSet.ts
Outdated
| }, | ||
| [rerender] | ||
| ); | ||
| return [get, set]; |
There was a problem hiding this comment.
Why not to use useSafeState, useSyncedRef and useCallback?
Besides this code will work - id prefer native state holder
There was a problem hiding this comment.
Sorry, I don't understand here.
id prefer
nativestate holder
what is it means here
There was a problem hiding this comment.
either useState nor useSafeState update the value immediately. That why this hook is need and useRerender is used in setter to control UI updates.
How useSyncedRef can be used here
There was a problem hiding this comment.
This implementation breaks 'state flow' of react, and will break in strict mode, 4ex.
What i propose is to implement hook like this:
export function useGetSet<S>(
initialState: IInitialState<S>
): [get: () => S, set: Dispatch<SetStateAction<S>>] {
const [state, setState] = useSafeState(initialState);
const stateRef = useSyncedRef(state);
// eslint-disable-next-line react-hooks/exhaustive-deps
return [useCallback(() => stateRef.current, []), setState];
}There was a problem hiding this comment.
This implementation doesn't return latest value in getter. /
There was a problem hiding this comment.
But how to write test, since I don't know when will react state change
There was a problem hiding this comment.
Just wrap all set calls with act method of testing library.
There was a problem hiding this comment.
Also, PR lack of readme changes.
There was a problem hiding this comment.
Just wrap all
setcalls withactmethod of testing library.
it('should set new value', () => {
const { result } = setUp('foo');
const [get, set] = result.current;
act(() => set('bar'));
const currentValue = get();
expect(currentValue).toBe('bar');
});
This test cannot pass /
|
|
||
| act(() => { | ||
| jest.runAllTimers(); | ||
| }); |
There was a problem hiding this comment.
it is way overcomplicated, why not to simply set state inside act?
act(()=>{
set();
set();
set();
})
|
🎉 This issue has been resolved in version 14.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What new hook does?
Port of
useGetSetin react-useChecklist
Port remaining hooks from
react-use#33