Thoughts on Context about overmind HOT 12 CLOSED

Very nice summary!! 👍

Solution (1)
Even though this is straight forward, I do not think it is the way to do typing. It would be like doing:

const MyComponent = (props: Props) => ()

Where you should do:

const MyComponent: React.SFC<Props> = (props) => ()

Now React know that MyComponent is a component, not a function with arbitrary typing on its argument. The same should be for operations. Overmind benefits from knowing that the function you compose in is of an actual operation type, not just an arbitrary function with some typing on it. "Typing on the left, implement on the right". Also you get more help in terms of expected return type etc.

Solution(2)
This is what we did previously as well. It just really annoys me that we have to do it that way, which is why the "hack" came up as a suggestion :) What I like here though is that you have improved the typing since last time. So copy pasting this feels better than the previous one :)

Solution(3)
It is really good to have the suggestion up here, though yeah... I feel this makes Typescript define the API, instead of Typescript just being sprinkled on top of existing API.

You know... my conclusion after seeing this is that I think the cleanup you did with types in solution2 has made it OK :D Just copy paste that typing and you are good to go. That is the most correct way to do it for sure.... and who knows, maybe namespaces will get generics at some point. Then we can clean it up even more.

So yeah, I vote for solution2 as the one and only solution :)

from overmind.

Also regarding type naming conventions, I don't really get why some types have T prefix and others have I.

I think all exposed types that are used to extract another type (like TAction --> Action) deserve a T and all other types should be simple or internal and have no prefix except for 'IApp' which is the default App type used when none is provided.

Not really sure the IApp and declare module hack stuff really helps. If we can import Action from overmind we can as well import it from our ./types or ./index file I think. It is less magical and avoids problems with declare module "overmind" when this thing gets included in a library by mistake.

So safer = no IApp as default.

from overmind.

We actually did have explicit typing in the beginning of Overmind, but at the end it required you to copy paste about 30 lines of Typescript. It was the same 30 lines for every app. It felt utterly unnecessary, which is why this "hack" arrived.

Most of the problem is the operations. There is a specific type for every operation, but you can not say: "For all operations use this state type and this effects type", you have to type each one of them. As namespaces can not have generics.

So I do not disagree what explicit typing would be the right way to do it, but currently it is really scary to see that big list of typing you have to copy paste into your code to make it work.

Though if you find a way to reduce the amount of initial typing setup required, but still be explicit I am onboard with that for sure! :)

from overmind.

There are actually multiple things to be discussed:

1. Using the notion of Context instead of Effects
2. Using BaseContext (Context without value) instead of State and Effects in types (Action, Compose, etc)
3. Use IApp as generic default. Since we no longer have modules in there, it is possible.

On my side, I'll refactor the track code so that it can be typed and exported from a single place for the whole app.

With this we have TAction from overmind for actions and connect for components. It should be two lines of typings to do this in app.ts.

@christianalfoni Tell me what you think of these different points and I'll make a PR (with changes to typescript todomvc).

from overmind.

@gaspard Could you give an example of how the typing for operations would look like? As I understand it would look something like:

const doThis: Operation.Map<BaseContext, string, string>

?

from overmind.

Instead of typing each operation, we can type a single function and it just works (the function is a pass through):

import { operation } from 'app'

const doThis = operation.map<string, string>( context => 'foo' )

from overmind.

aha, cool, though you have an example of a complete app file? Just need to see a complete config for an app so I am sure we are merging all our knowledge here :)

from overmind.

If I remember correctly we have already talk about function as a way to type (for actions maybe ?) and one of the problem was that these functions make no sense if you don't have TS and so you end up with a different way of doing things depending on the usage of TS or not... While with current typings you just add type to your codebase and nothing more...
That being said I do think typings with functions is worth the try just to see how it feels...

from overmind.

Oh, I misread the example... yeah, we have been through factories before and I am very unsure about this.

From a critical perspective:

1. We are adding API surface that has nothing to do with the runtime. It is there to help development. It would be like adding factories to improve linting. It makes absolutely no sense

2. It makes even less sense for people who does not use TypeScript. They have a factory that does absolutely nothing, not even in development. Have to remember that the experience with Overmind not using TypeScript is pretty amazing. There are absolutely no imports defining your logic, everything is just plain functions

3. We are moving away from "typing on the left, implementation on the right", to "inference by factory". Not saying that is bad, just saying it is a different take on the API, which needs to be consistent throughout

From an encouraging perspective:

1. The conceptual approach for the API is "factory for inference", meaning that everything should be a factory. (mutations, derived, reactions etc.) The only typing is the generics for the factory. Which means somewhat less typing and probably better errors

2. We have to provide two different APIs, which the current documentation framework already supports.

With plain JS you would just say:

const doThisMapping = (context) => {}

And with TS you would say:

const doThisMapping = operations.map<string, string>(context => {})

So basically we have achieved everything as plain functions now, which is a really nice base. Then we add typing factories on top of those to reduce amount of typing and get more inference.

Summary
I am not convinced using factories is the right move. Now we have a plain functional JavaScript API where types are an addition to the existing API. That is how Typescript should work. What we are talking about here is flipping it completely around, pleasing Typescript by changing the API. That is soooo wrong in my book. That said, we live in a crazy world where types has been added to a language not designed for types.

But there is nothing wrong adding this typing + factories in parallel to what we have now? If we disagree on this we can just implement these factories and typing next to what we have now, as it is just something "on top" anyways. Try it out :)

from overmind.

So here are some ideas:

Action and context types

This is required in any case (for explicit typing, library authors, etc):

// This is the extra work for typing the app:
export type App = TApp<MyConfig>
export type Action<Value, ReturnValue = Value> = TAction<App, Value, ReturnValue>
export type Context<Value> = TContext<App, Value>

Operation types

Solution (1) explicit context typing (simplest)

This is probably the best solution for when we do not have huge apps/libraries.

// Does not provide much information on what `doThis` does.
const doThis = ({state}: Context<string>) => state.foo

Solution (2) namespace typing (most complex)

This may be a good solution for big apps and libraries. We can use documentation to help people copy/paste this.

export namespace Operation {
  export type Map<Value, ReturnValue = Value> = (
    ctx: Context<Value>
  ) => ReturnValue
  export type Filter<Value = any> = (ctx: Context<Value>) => boolean
  export type When<Value = any> = (ctx: Context<Value>) => boolean
  export type Run<Value = any> = (ctx: Context<Value>) => void
  export type Fork<Value = any> = (ctx: Context<Value>) => string
  export type Attempt<Value, ReturnValue> = (
    ctx: Context<Value>
  ) => ReturnValue
}

// Provides good information on what the function does:
const doThis: Operation.Map<string> = ({state}) => state.foo

Solution (3) with function wrapping (hack)

I don't like this solution as it does not provide enough added value.

export const operation = new TypedOperaion<App>()

// Does not add information to function when exported
const doThing = operation.map<string>(({state}) => state.foo)

from overmind.

Same here, solution2 sound like the good one !

from overmind.

I agree that TypeScript ugliness shouldn't stop Overmind from doing The Right Thing (when is it not ugly anyway?). I have limited TypeScript, but it seems people have accepted they will need to do something in order for a new dependency's types to work.

That said, could all this be put together in a way that Overmind is TypeScript compilable without that wall of copy and paste, and then useful with it?

from overmind.