TypeScript definitions

[corydeppen]

Would you be willing to include a TypeScript declaration file in this repo and publish it as part of the package? I'm interested in using this library in a TypeScript project and would be willing to work on a PR that uses the existing Flow types as the basis for the TS definitions.

[aweary]

I would love to have a TypeScript definition! I need to get a working library definition for Flow too (right now the types are internal) so the timing is great.

[samhh]

These aren't production-ready, but they're a start.

declare module 'react-copy-write' {
	import { Component } from 'react';

	interface ProviderProps {
		children: JSX.Element | JSX.Element[];
		initialState?: object;
	}

	class Provider extends Component<ProviderProps, any> {}

	type MutateFn<T> = (draft: T) => void;
	type Mutator<T> = (mutator: MutateFn<T>) => void;

	// "any" due to selector
	type RenderFn<T> = (state: T | any, mutate: MutateFn<T>) => JSX.Element | JSX.Element[];

	interface ConsumerProps<T> {
		selector?: (state: T) => any;
		render?: RenderFn<T>;
		children?: RenderFn<T>;
	}

	class Consumer<T> extends Component<ConsumerProps<T>, any> {}

	function create<T extends object>(state: T): {
		Provider: new() => Provider,
		Consumer: new() => Consumer<T>,
		mutate: Mutator<T>,
	};

	export default create;
}

[davej]

@SamHH Would you consider publishing these somewhere so that those of us using typescript can iterate on them?

[samhh]

@davej I've created a formal type definition here that can be merged into DefinitelyTyped via PR once they're a bit further along. Feel free to contribute there until it's ready for PR; speaking of which, I've not submit anything to DT before so any feedback on that directory is helpful, particularly with regards to the "test".

[aweary]

Im not a TypeScript user. Is it more common for type definitions to live in Definitely Typed or the repo itself?

Happy to do do either.

[davej]

@aweary I'm only starting with typescript but I think it's better to include them in the repo itself if possible. DT is for the community to provide an escape hatch when a library decides not to include type definitions for whatever reason.

[davej]

I've not submit anything to DT before so any feedback on that directory is helpful

@SamHH Either have I! This is literally my first week using TypeScript (starting a new greenfields project). I'm not sure if I can be much help but I'll try my best. :-)

[samhh]

@aweary @davej The TypeScript documentation here says that if the project is not written in TypeScript then it's better for the types to be stored in the DefinitelyTyped repo.

[davej]

@SamHH Ah ok. I was basing my answer on what I read on StackOverflow but TypeScript documentation is a much more canonical source.

[aweary]

Note that the API has gone through some changes since @SamHH's original attempt.

• mutate is no longer passed to the render callback of consumers
• selector has been renamed select and must be an array of selector functions
• The render callback for consumers no longer pass the result of the selectors as an array. Instead they are now arguments to the function.

A couple other things:

• Does using any with the selector cause it to lose the types, or can it still infer based on what the selector returns? Maybe a generic is more appropriate?
• Can you define "conditional mutability" in TypeScript? For example, in mutate the state is mutable, but in the Consumer callback the state is not. It'd be awesome if we can represent that with types

[samhh]

@aweary Yes, using any means you're basically telling the compiler that you don't know what it is. I'm unsure how to solve this, any feedback from more experienced TS folk is obviously welcome.

With regards to conditional mutability, I think we could use Readonly generic and its variants to accomplish that?

[samhh]

Made the PR here with the help of @davej. Hopefully they'll be up soon-ish!

[samhh]

It's merged and should be available on npm shortly. :confetti_ball:

Edit: It's up! :confetti_ball:

[aweary]

Thanks you both @SamHH @davej! I definitely want to figure out a way to avoid any but this is a great start. I'll share the flow definitions we have internally soon, maybe that will be a good reference.

[samhh]

As I alluded to in the comments of the type definition I had two ideas to solve the any issue but both involve features not yet offered by TypeScript.

1. ConsumerProps union. This doesn't work because of type-widening; the absence of the select property does not disqualify the select-less interface. I tried to instead make the select-less interface define select as undefined, but that didn't work either.

2. Variadic kinds. My memory of what didn't work here is a bit rusty, but as I remember it: It would involve a few more changes, but in essence this plus conditional inferrence could work. My stumbling block was my inability to map a tuple of functions into a tuple of return types.

Of course, it's possible that there's a better way that would work today, but if so it's totally passed me by! :stuck_out_tongue:

[davej]

@SamHH: I'm seeing an error when using the second param (readable state) of the mutate function.

For example, in the code below:

// store.ts
export interface IApp {
  name: string;
  url: string;
  iconUrl?: string;
  icons?: UploadFile[];
}

// actions.ts
export const updateApp = (props: Partial<IApp>) =>
  mutate((draft, state) => {
    const appIndex = state.selectedApp;
    const app = state.apps[appIndex];
    // ▼▼ TS ERROR ON LINE BELOW ▼▼
    draft.apps[appIndex] = {
      ...app,
      ...props
    };
  });

The error that I'm getting is the following:

Type 'DeepReadonly<UploadFile[]>' is not assignable to type 'UploadFile[]'.

I could change the mutate function to just use the first param (draft) and read from that but it would be less performant because it's going through the Proxy object. Any ideas on how we could update the definition to get this working?

edit: Definition for UploadFile is here: https://github.com/ant-design/ant-design/blob/master/components/upload/interface.tsx#L13

[aweary]

@davej not sure about the TypeScript error, but the action you have there is doing more updates than necessary. You don't need to spread app or create a new object. Just mutate the existing one!

export const updateApp = (props: Partial<IApp>) =>
  mutate((draft, state) => {
    const appIndex = state.selectedApp;
    for (const key in props) {
        draft.apps[appIndex][key] = props[key]
     }
  });

[davej]

@aweary: That's true. I guess the issue that I was pointing out still stands though. There are cases where you may wish to assign something from state to draft and the current Typescript definitions output an error when doing that with complex objects.

[samhh]

There is unfortunately not yet a DeepReadonly in the standard library (see here), so I added one myself that has unfortunately proven naive. I'm making a PR to change it to Readonly; it will only prevent top-level mutation, but it does fix the above issue.

Edit: PR up.

[samhh]

Back to the any issue, I still haven't cracked it but I think this is better than what we have now. Give it a try and let me know how it works for you (chuck it inside a declare module 'react-copy-write' {} in a .d.ts file after uninstalling @types/react-copy-write)!

import { Component } from 'react';

type MutateFn<T> = (draft: T, state: Readonly<T>) => void;
type Mutator<T> = (mutator: MutateFn<T>) => void;

type SelectorFn<T, U> = (state: T) => U;

// Below probably works 3.0+
// type RenderFn<T extends any[]> = (...state: Readonly<T>) => JSX.Element | JSX.Element[] | null;
type RenderFn<T extends any[]> = (
    datum1: Readonly<T[0]>, datum2: Readonly<T[1]>, datum3: Readonly<T[2]>, datum4: Readonly<T[3]>, datum5: Readonly<T[4]>, datum6: Readonly<T[5]>, datum7: Readonly<T[6]>, datum8: Readonly<T[7]>, datum9: Readonly<T[8]>, datum10: Readonly<T[9]>,
) => JSX.Element | JSX.Element[] | null;

interface ConsumerProps<T, U extends any[]> {
    // Below probably works 3.0+
    // select?: [SelectorFn<T, U[0]>?, SelectorFn<T, U[1]>?, SelectorFn<T, U[2]>?, SelectorFn<T, U[3]>?, SelectorFn<T, U[4]>?, SelectorFn<T, U[5]>?, SelectorFn<T, U[6]>?, SelectorFn<T, U[7]>?, SelectorFn<T, U[8]>?, SelectorFn<T, U[9]>?];
    select?:
        [SelectorFn<T, U[0]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>, SelectorFn<T, U[5]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>, SelectorFn<T, U[5]>, SelectorFn<T, U[6]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>, SelectorFn<T, U[5]>, SelectorFn<T, U[6]>, SelectorFn<T, U[7]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>, SelectorFn<T, U[5]>, SelectorFn<T, U[6]>, SelectorFn<T, U[7]>, SelectorFn<T, U[8]>] |
        [SelectorFn<T, U[0]>, SelectorFn<T, U[1]>, SelectorFn<T, U[2]>, SelectorFn<T, U[3]>, SelectorFn<T, U[4]>, SelectorFn<T, U[5]>, SelectorFn<T, U[6]>, SelectorFn<T, U[7]>, SelectorFn<T, U[8]>, SelectorFn<T, U[9]>];
    render?: RenderFn<U>;
    children?: RenderFn<U>;
}

declare class Consumer<T, U extends any[]> extends Component<ConsumerProps<T, U>> {}

interface ProviderProps<T> {
    children: JSX.Element | JSX.Element[];
    initialState?: Partial<T>;
}

declare class Provider<T> extends Component<ProviderProps<T>> {}

declare function create<T extends object>(state: T): {
    Provider: new() => Provider<T>,
    Consumer: new<U extends any[] = [T]>() => Consumer<T, U>,
    createSelector: <U extends SelectorFn<T, any>>(selector: U) => U,
    mutate: Mutator<T>,
};

export default create;

I've added a generic to the Consumer component. This type represents the array of arguments you're going to receive back in your render functions, so it defaults to [T] where T is your state/store object. If you pass in any selectors, then you'll need to manually type the generic. An example is below:

import * as React from 'react';
import { render } from 'react-dom';
import createStore from 'react-copy-write';

const { Provider, Consumer, mutate, createSelector } = createStore({
    user: {
        avatar: {
            src: '',
        },
        age: 42,
    },
});

// Create reusable selectors for optimal performance
const selectAvatar = createSelector(state => state.user.avatar.src);
const selectAge = createSelector(state => state.user.age);

// Use state for lookup of preexisting data instead of draft (per underlying
// immer library)
const incrementAge = () => mutate((draft, state) => {
    draft.user.age = state.user.age + 1;
});

const App = () => (
    <Provider>
        <div>
            <Consumer<[ReturnType<typeof selectAvatar>, ReturnType<typeof selectAge>]> select={[selectAvatar, selectAge]}>
                {(avatarSrc, age) => (
                    <>
                        <img src={avatarSrc} />

                        <p>User is {age} years old</p>
                    </>
                )}
            </Consumer>

            <Consumer render={state => (
                <button onClick={incrementAge}>Increment age</button>
            )} />
        </div>
    </Provider>
);

render(<App />, document.body);

Where I've gone for the very safe [ReturnType<typeof fn>], you could instead do simply [string, number, etc] for brevity.

There are some caveats to this approach.

1. You're forced to manually type your components. This is annoying because the type information is there, I just can't figure out any way to access it within TypeScript today.
2. You cannot use more than 10 selectors without reverting back to needing to assert your types. That said, who is using 10 selectors? :bell:
3. If you for example use two selectors and you type them accordingly, any further arguments you attempt to extract in your render function will wrongly infer type as a union of the preceding values e.g. args [number, string] would infer any further arguments as number | string, which appears to be the default behaviour in TypeScript when accessing an array/tuple at an undefined index (can be simulated with MyArray[number].
4. The type definition is much less readable, but I've left some commented alternatives in that I believe will work in TS 3.0+.

Oh, and I'm not sure because I hadn't yet used any selectors myself, but I think the createSelector types are much improved now.

[samhh]

^^ Please try and feedback the above before I submit a PR, don't want to introduce any more instability (a la the DeepReadonly issue). :smile:

[davej]

@SamHH: Using it now, I've made a note to report back after a few days of use.

[davej]

@SamHH: So far, so good. Anything in particular that I should be looking out for?

[samhh]

@davej Aside from generally noticing any potential regressions, I'm just wondering if for others those specified caveats are worthwhile. I think they are but I want feedback before I thrust them upon everyone!

[davej]

@SamHH:

Well I had trouble doing the following (based on your example):

const selectApp = createSelector(state => state.apps[state.selectedApp]);

const BuildContainer: React.SFC = () => (
  <Consumer<[ReturnType<typeof selectApp>]> select={[selectApp]}>
    {(app) => <Build app={app} />}
  </Consumer>
);

The <[ReturnType<typeof selectApp>]> signature seems to cause a build error? I'm using Typescript 2.9.2.

[samhh]

VSCode isn't giving me any errors, what's the build error? @davej

[davej]

I'm not getting a build error, just "Failed to compile" from typescript and tslint is passing. I'm using @babel/preset-typescript and react-app-rewire-typescript-babel-preset so potentially it is a bug in one of those libraries?

[samhh]

Hmm, I think that would suggest an issue with those; I've never personally seen TypeScript fail without telling you why, however verbose or cryptic.

[davej]

Yes, it does that on all errors. Usually TSLint will catch the error so it hasn't really been an issue for me so far.

Created an issue on the repo of the boilerplate I'm using to see if I can get to the bottom of it. https://github.com/Jayphen/Boilerplate-CRA2-Typescript-Emotion/issues/2

[davej]

Ok I downgraded to [email protected] and now I am getting the following error:

./src/steps/6-Build.tsx
Syntax error: {snip}/src/steps/6-Build.tsx: Unexpected token (153:11)

  151 | const selectApp = createSelector(state => state.apps[state.selectedApp]);
  152 | const BuildContainer: React.SFC = () => (
> 153 |   <Consumer<[ReturnType<typeof selectApp>]> select={[selectApp]}>
      |            ^
  154 |     {(app: IApp) => <Build app={app} />}
  155 |   </Consumer>
  156 | );

Perhaps this syntax just isn't supported in babel react for some reason. Do you know if there is an alternative syntax for doing this?