-
-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add optional second parameter to useCreateCtx with custom ctx creation function #1012
Comments
So, |
@MOZGIII there is no need for additional API, base hook would work fine with new parameter. I think, it should be like this: export const useCreateCtx = <T extends typeof createCtx = typeof createCtx, C extends Ctx = ReturnType<T>>(
extension?: Fn<[C]>,
createFn: T = createCtx as T,
): C => {
const ctxRef = React.useRef(null as null | C)
if (!ctxRef.current) {
ctxRef.current = createFn() as C
extension?.(ctxRef.current)
}
return ctxRef.current
} |
Too much generics imo, I'd say we need either |
@MOZGIII it is required for better type safety, but you can try another implementation |
type CreateCtxFn = <C extends Ctx>() => C;
export const useCreateCtx = <T extends CreateCtxFn = typeof createCtx>(
extension?: Fn<[ReturnType<T>]>,
createFn: T = createCtx as T,
): C => {
const ctxRef = React.useRef<null | ReturnType<T>>(null)
if (!ctxRef.current) {
ctxRef.current = createFn()
extension?.(ctxRef.current)
}
return ctxRef.current
} Ok, how about this? Might need to test it, just wrote it here... But the idea is similar, yet different. Here we disallow separating the The only thing I'm still worried about is this |
Also, why have this signature in the first place, if we could just do this: type CreateCtxFn = <C extends Ctx>() => C;
export const useCreateCtx = <T extends CreateCtxFn = typeof createCtx>(
createFn?: T = createCtx as T,
): C => {
const ctxRef = React.useRef<null | ReturnType<T>>(null)
if (!ctxRef.current) {
ctxRef.current = createFn()
}
return ctxRef.current
} The Plus, it looks like we could just do |
Discussed in #994
Originally posted by MOZGIII December 30, 2024
I've been doing some development with reatom, and figured we need a way to create a test ctx on a per-page basis.
We have some dev-routes with pages that need mocking - sort of like storybooks but at the same app code as the rest of the app.
It would also be fine to have a just more generic way to create and destroy the contexts, so that
useCreateCtx
doesn't invoke the thecreateCtx
itself but rather allows the user to do so.The second variant should work better as it does not have direct dependency on both
react
and a@reatom/testing
- which would make it quite puzzling where to place this code.The text was updated successfully, but these errors were encountered: