Closed dmicic closed 5 years ago
Merging #28 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #28 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 6 +1
Lines 56 66 +10
Branches 11 11
=====================================
+ Hits 56 66 +10
Impacted Files | Coverage Δ | |
---|---|---|
src/EventBus.ts | 100% <ø> (ø) |
:arrow_up: |
src/react.ts | 100% <100%> (ø) |
:arrow_up: |
src/useBusState.ts | 100% <100%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 696bcda...30fa135. Read the comment docs.
I tried to get rid of the second parameter in the useBusState function but didn't manage it in Typescript.
Not sure there is an easy solution for the typescript issue as we need a way to define the event we are using and subscribe to that event. Unless we can avoid specifying the event in the subscribe function.
Also I recommend using useLayoutEffect
instead of useEffect to avoid the lost event problem.
@ryardley I did the following changes:
Regarding:
Not sure there is an easy solution for the typescript issue as we need a way to define the event we are using and subscribe to that event. Unless we can avoid specifying the event in the subscribe function.
I was hoping to somehow extract the event name out of the generic function parameter. But as mentioned, I couldn't find a way.
export function useBusState<E extends BusEvent = BusEvent>(
initState: E["payload"] | undefined,
event: E["type"] // Try somehow to get the event name as the default value?
): [ E["payload"], Dispatch<SetStateAction<E["payload"]>>] {
Apart from docs. Looks good to me 🚀
The useBusReducer is very handy if you centrally manage your state. However, it's a overkill for simple scenarios where your component for example listens to a specific event. The useBusState should reduce the typing to a minimum.
@ryardley Do you see this a fit for ts-bus?