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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TypeScript #36

Merged
merged 5 commits into from Aug 23, 2018

Conversation

Projects
None yet
3 participants
@diegohaz
Copy link
Owner

diegohaz commented Aug 22, 2018

This PR rewrites the library in TypeScript. I've also removed the mount helper as it was an experimental feature that was already hard to maintain. I'll come with another solution for that later (if people demand).

So, I started playing around with TypeScript just a few days ago, and typing Constate was a big challenge. A friend of mine already told me that it was hard to type (properly). Now I know 馃槄! I have some experience with typings with Swift and Flow, so I haven't started from zero.

I'll copy and paste the documentation I wrote in the README here:

TypeScript

Constate is written in TypeScript and exports many useful types to help you. When creating a new container, you can start by defining its public API. That is, the props that are passed to the children function:

interface State {
  count: number;
}

interface Actions {
  increment: (amount?: number) => void;
}

interface Selectors {
  getParity: () => "even" | "odd";
}

interface Effects {
  tick: () => void;
}

In computer programming, it's a good practice to define the API before actually implementing it. This way, you're explicitly saying how the container should be consumed. Then, you can use handful interfaces exported by Constate to create your container:

import { ActionMap, SelectorMap, EffectMap } from "constate";

const initialState: State = {
  count: 0
};

const actions: ActionMap<State, Actions> = {
  increment: (amount = 1) => state => ({ count: state.count + amount })
};

const selectors: SelectorMap<State, Selectors> = {
  getParity: () => state => (state.count % 2 === 0 ? "even" : "odd")
};

const effects: EffectsMap<State, Effects> = {
  tick: () => ({ setState }) => {
    const fn = () => setState(state => ({ count: state.count + 1 }));
    setInterval(fn, 1000);
  }
}

Those interfaces (e.g. ActionMap) will create a map using your State and your public API (e.g. Actions).

If you're using VSCode or other code editor that supports TypeScript, you'll probably have a great developer experience. Tooling will infer types and give you autocomplete for most things:

Example

Then, you just need to pass those maps to Container:

const Counter = () => (
  <Container
    initialState={initialState}
    actions={actions}
    selectors={selectors}
    effects={effects}
  >
    {({ count, increment, getParity, tick }) => ...}
  </Container>
);

It'll also provide autocomplete and hints on the public API:

Example

If you're building a composable container - that is, a component without children that receives props -, you can define your component as a ComposableContainer:

import { Container, ComposableContainer } from "constate";

const CounterContainer: ComposableContainer<State, Actions, Selectors, Effects> = props => (
  <Container
    {...props}
    initialState={{ ...initialState, ...props.initialState }}
    actions={actions}
    selectors={selectors}
    effects={effects}
  />
);

Then, you can use it in other parts of your application and still take advantage from typings. ComposableContainer will handle them for you:

const Counter = () => (
  <CounterContainer initialState={{ count: 10 }} context="counter1">
    {({ count, increment, getParity, tick }) => ...}
  </CounterContainer>
);

ComposableContainer doesn't accept other actions, selectors and effects as props. That's because, as of today, there's no way for TypeScript to dynamically merge props and infer their types correctly.

There're also useful interfaces for lifecycle methods. You can find them all in src/types.ts.

cc @codyaverett @stereobooster @Thomazella @gtkatakura @hnordt

diegohaz added some commits Aug 22, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 22, 2018

Codecov Report

Merging #36 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #36   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      4    -2     
  Lines         167    115   -52     
  Branches       45     31   -14     
=====================================
- Hits          167    115   -52
Impacted Files Coverage 螖
src/utils.ts 100% <100%> (酶)
src/Context.ts 100% <100%> (酶)
src/Container.tsx 100% <100%> (酶)
src/Provider.tsx 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 47111fd...e598cca. Read the comment docs.

@diegohaz

This comment has been minimized.

Copy link
Owner Author

diegohaz commented Aug 22, 2018

@gtkatakura came up with an idea of inferring the public API (children props) by the implementation (action map) instead of defining the public API first and inferring the implementation.

That was the first thing I tried to do, but for some reason I can't remember it didn't work well.

I particularly like the benefits of defining the API first, it helps organizing things in my mind, but it also makes everything more verbose.

I'll try to do what @gtkatakura suggested and compare both approaches. Maybe there's also a solution that covers both cases.

@Thomazella

This comment has been minimized.

Copy link

Thomazella commented Aug 22, 2018

The current examples are very readable. I think defining the API first helps with that.

@stereobooster stereobooster referenced this pull request Aug 22, 2018

Closed

[WIP] Flow #35

@diegohaz

This comment has been minimized.

Copy link
Owner Author

diegohaz commented Aug 23, 2018

I tried some things to make it work the other way around. But all the solutions have broken something and they were complex to the point that they became unmaintainable.

Thinking more about it, I don't think that's a good idea to infer the API with the implementation. We'll loose a lot of inference while developing, which leads to a poor developer experience.

Defining the API first is pretty much like defining component props first. It helps creating the container to meet the API and works as a nice documentation for those opening the file.

I still want to improve it, but I'm gonna merge this for now.

@diegohaz diegohaz merged commit de1cd09 into master Aug 23, 2018

4 checks passed

codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 47111fd
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@diegohaz diegohaz deleted the typescript branch Aug 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment