Simplified typing
christianalfoni opened this issue ยท 20 comments
Dear Overminders!
I have one thing I really want to do and that is to clean up and simplify the typing of Overmind. It was a long process figuring out how to do it and with acquired knowledge through all this work and later projects I would like to do this:
import { IContext } from 'overmind'
const config = { ... }
export interface Context extends IContext<{
state: typeof config.state,
actions: typeof config.actions,
effects: typeof config.effects
}> {}
There will be no more implicit typing, the above is what everybody will need to do. This is how you type the different things:
const myAction = (context: Context) => {}
const myActionWithPayload = (context: Context, payload: string) => {}
// Now we support generics
const myActionWithPayload = <T extends TEvent>(context: Context, payload: T) => {}
const myOperator = map((context: Context, value: string) => {})
const state: State = {
someDerived: derived((state: State, rootState: Context["state"]) => {})
}
And I believe that is it. This will need to be a breaking change to actually clean up the internal typing of Overmind. I was just wondering how you feel about this change, do you consider refactoring your apps to this typing too much work? ๐
As part of this change the operators are also extremely simplified, and best of all... you can just compose any action directly in as an operator.
const someAction = ({}: Context, value: string) => {}
const operator = pipe(
someAction,
wait(500)
)
It basically means operators finally are interoperable with plain actions. Which also means we do not need mutate
, map
and run
anymore. You can just give it a plain action to do all those things. We rather have operators that goes beyond what a normal action can do. Like debounce
, wait
, fork
, waitUntil
etc.
This is such a huge improvement!
Looks like a great improvement and it doesn't look like a lot of work to refactor our app.
Looks good, will there be errors if the signature of actions doesn't match (Context, Payload). f.ex. what would happen if an action similar to this is defined:
const myAction = (context: string) => {}
@jmattheis Good question!
Yes, when the actions are attached to Overmind it expects the signature of: ยด(context: IContext)` , so you will get the error where you instantiate Overmind rather than where you define the action ๐
So a summary of what is being moved into next
now is:
New typing, no more implicit typing:
import { IContext } from 'overmind'
const config = { ... }
export type Context = IContext<{
state: typeof config.state,
actions: typeof config.actions,
effects: typeof config.effects
}>
Type actions:
const myAction = (context: Context) => {}
const myActionWithPayload = (context: Context, payload: string) => {}
Operator changes:
- Now an operator is interoperable with actions, meaning you can just do:
const whatever = pipe(
(context: Context, input: string) => input.toUpperCase()
)
-
Due to interoperable actions, the
map
,mutate
andrun
operators has been removed. Just use plain action signature -
forEach
operators has also been removed as the typing never worked there (This might come back, have to research some more) -
fork
has new signature:
pipe(
// "type" is a property on the object on the pipe
fork('type', {
FOO: (context: Context, event) => {
event // Is now typed as FooEvent
},
BAR: (context: context, event) => {
event // Is now typed as BarEvent
}
})
)
parallel
now actually returns an array to the pipe with the results of each operator
React hooks
useOvermind
is removed in favour of explicit hooks (more efficient and cleaner)useState
hook now has an added signature
const ItemComponent = ({ id }) => {
// The state you access within the callback is not tracked,
// only the result and further access is tracked. Meaning this
// component will only track the item, not changes to "items"
const item = useAppState(state => state.items[id])
}
@christianalfoni can you give full example of the new typings feature with a simple state and action? because when I try it like this typescript complains:
import { IContext } from 'overmind'
import { createActionsHook, createStateHook } from 'overmind-react'
export type StateType = {
value: string | null
}
export const state: StateType = {
value: null
}
const changeState = ({ state }: Context, value: any) => {
state.value = value
}
export const config = {
state,
actions: {
changeState
},
}
export type Context = IContext<{
state: typeof config.state,
actions: typeof config.actions,
effects: typeof config.effects
}>
export const useAppState = createStateHook<Context>();
export const useActions = createActionsHook<Context>();
'config' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.ts(7022)
'state' is referenced directly or indirectly in its own type annotation.ts(2502)
'changeState' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.ts(7022)
Hi @ssijak , when you write this in the same file you will get this error. If you rather do:
export type Context = IContext<{
state: typeof state,
actions: typeof actions,
effects: typeof effects
}>
Then it works within the same file. typeof config.*
only works across files. I guess this is some TypeScript bug, or just how it works. But typically you would not put it in the same file and there are no examples in the documentation. BUT! I totally see why you tried ๐
Thanks for the quick answer! It works now, it does not complain. But I noticed some possible issues around actions. I am a relative typescript noob so maybe I'm talking nonsense but:
If I type action parameters as any like :
const changeState = ({ state }: Context, value: any) => {
state.value = value
}
than c.changeState bellow is inferred to ()=>void
type
export const useActions = createActionsHook<Context>();
const c = useActions()
c.changeState()
Thanks for testing! Doing any
will also match void
I guess, which is how actions identify "no payload". I would strongly argue that any
should not be used as a typed payload, but it is confusing for sure ๐
any
is typically used for unknown objects. Cause if it was like a string, number etc. you would actually type it. So doing {}
instead will probably fix it, as it matches any object, but not void
But this is good for documentation for sure!
Thanks. I encountered one more possible issue. I do not find OnInitialize
exported from overmind in the next tag. Is it removed or?
edit: So I should just use onInitializeOvermind action?
@christianalfoni This is all coming in V28 right? Do you have a rough release date in mind?
There has been a little bit lack of time lately (Blame the baby mostly ๐ ), but also at codesandbox.io we are hiring a few people and it has also taken up a lot of my time.
But if you look at the issue list now I have marked some issues as NEXT RELEASE
, those needs to be fixed. Good thing is that our codesandbox.io PR, codesandbox/codesandbox-client#5517, is now running green and should be merged shortly. Which was also a blocker for the next release.
So yeah, I hope to get some time soon to get through the pending issue and get that release out ๐
@christianalfoni I wanted to try out these new typings with my current namespaced config where my state and actions are placed in a separate folder. However in order to access the new Context
type it needs to be imported in the actions file which would cause a cyclic dependency, how would I go about avoiding this? I couldn't figure it out from your examples.
overmind/index.ts
import { IContext } from 'overmind'
import { namespaced } from 'overmind/config'
import * as status from './status' //<---- cyclic dependency
export const config = namespaced({
status,
})
export type Context = IContext<{
state: typeof config.state
actions: typeof config.actions
}>
overmind/status/index.ts
import { state } from './state'
import * as actions from './actions'
export { state, actions }
overmind/status/actions.ts
import { Context } from '..' // <---- cyclic dependency
export const updateState = (context: Context, payload: string) => {}
Thanks for an awesome library!
@mattiasjosefsson Move config into a separate file :-)
@eirikhm Aaah of course! Thanks!
@eirikhm Finally got around to test it but it doesn't help to move config into a separate file because actions
are still going to be referenced by Context
in overmind.ts
@mattiasjosefsson something like this:
overmind/config.ts
import { merge, namespaced } from "overmind/config"
import { state } from "./state"
import * as actions from "./actions"
import * as effects from "./effects"
import * as status from "./status"
export const config = merge(
{
state,
actions,
effects,
},
namespaced({
status,
})
)
overmind/index.ts
import {
createActionsHook,
createStateHook,
createEffectsHook,
} from "overmind-react";
import { IContext } from "overmind";
import { config } from "./config";
export type Context = IContext<{
state: typeof config.state,
actions: typeof config.actions,
effects: typeof config.effects,
}>
export type RootState = typeof config.state;
export const useAppState = createStateHook<Context>()
export const useActions = createActionsHook<Context>()
export const useEffects = createEffectsHook<Context>()
overmind/status/actions.ts
import { Context } from "../index"
export const updateState = (context: Context, payload: string) => {}
@eirikhm Could it be just eslint that complains when it's not really an issue? It will complain with the rule import/no-cycle
both for effects and actions that import the Context
type.
actions.ts
import { Context } from '.' // Dependency cycle via ./config:4 eslint(import/no-cycle)
export const updateState = (context: Context, payload: string) => {}
config.ts
import { state } from './state'
import * as actions from './actions' // Dependency cycle via .:1 eslint(import/no-cycle)
export const config = {
state,
actions,
}
index.ts
import { createActionsHook, createStateHook } from 'overmind-react'
import { IContext } from 'overmind'
import { config } from './config' // Dependency cycle via ./actions:2 eslint(import/no-cycle)
export type Context = IContext<{
state: typeof config.state
actions: typeof config.actions
}>
export type RootState = typeof config.state
export const useAppState = createStateHook<Context>()
export const useActions = createActionsHook<Context>()
state.ts
export type TestState = {
test: string
}
export const state: TestState = {
test: '',
}
@christianalfoni So there is no way to simplify writing actions? Duplicating export const
everywhere doesn't feel good. Ideally, for me, it would be object actions with local type definitions.
const actions = {
example (context: Context, params: { a: string, b: boolean }) {}
}
Right now if I try to write this way I get the error:
'actions' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.ts(7022)
@mattiasjosefsson no, it is not an issue with eslint. It is definitely a cyclic reference. I faced the same problem.
It doesn't matter if some portion of code gets moved into a separate file, because Context
type can be defined only when the full config object is merged together, and full config object includes actions, which require Context
to type their args.
What you could try to do is declaring the module to define the Context type
store/index.ts
import type { IContext } from 'overmind';
import {
createActionsHook, createEffectsHook, createStateHook, createReactionHook
} from 'overmind-react';
import { merge, namespaced } from 'overmind/config';
import * as settings from './settings';
const overmindStoreConfig = merge(
{
state: {},
actions: {},
effects: {}
},
namespaced({
settings
})
);
type OvermindConfig = IContext<typeof overmindStoreConfig>;
// here
declare module 'overmind' {
interface Context extends OvermindConfig {}
}
const useOvermindActions = createActionsHook<OvermindConfig>();
const useOvermindState = createStateHook<OvermindConfig>();
const useOvermindEffects = createEffectsHook<OvermindConfig>();
const useOvermindReaction = createReactionHook<OvermindConfig>();
export {
useOvermindEffects,
useOvermindActions,
useOvermindState,
useOvermindReaction,
overmindStoreConfig
};
settings/actions.ts
import type { IAction, Context } from 'overmind'; // and then import it like this
import type { State } from './state';
const updateSettings: IAction<State, void> = ({ state }: Context, settings) => {
state.settings = {
...settings
};
};
const onInitializeOvermind: IAction<Context, void> = ({ effects }, instance) => {
instance.reaction(
(state) => state.settings,
(settings) => effects.storage.save(settings),
{ nested: true }
);
};
export {
updateSettings,
onInitializeOvermind
};