Skip to content
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

Breaking change in produce typescript definition #288

Closed
1 task done
grassick opened this issue Jan 11, 2019 · 4 comments · May be fixed by dyna-dot/create-react-app#4 or hzhz/create-react-app#3
Closed
1 task done

Breaking change in produce typescript definition #288

grassick opened this issue Jan 11, 2019 · 4 comments · May be fixed by dyna-dot/create-react-app#4 or hzhz/create-react-app#3

Comments

@grassick
Copy link

  • Issue: Curried TS definition allows undefined to be passed as base
    • Version: 1.10.0
    • Expected behavior: produce currying definition which returns a function that takes a first argument of type Base (the way it used to work before 8666d32)
    • Observed behavior: produce currying definition that returns a function that takes Base | undefined as first parameter

The definition for the curried produce function used to be:

    /** Curried producer with no initial state */
    <State, Result = any, Args extends any[] = any[]>(
        recipe: (
            this: Draft<State>,
            draft: Draft<State>,
            ...extraArgs: Args
        ) => void | Result
    ): (base: State, ...extraArgs: Args) => void extends Result ? State : Result

It is now:

    /** Curried producer */
    <Default = any, Base = Default, Rest extends any[] = [], Return = void>(
        recipe: (
            this: Draft<Base>,
            draft: Draft<Base>,
            ...rest: Rest
        ) => Return,
        defaultBase?: Default
    ): (base: Base | undefined, ...rest: Rest) => Produced<Base, Return>

The fact that the base can be undefined in the returned function messes up existing typescript code that doesn't expect this.

@aleclarson
Copy link
Member

Nice catch. I somehow overlooked the | undefined when merging the two currying-related function signatures. 😅

@aleclarson aleclarson self-assigned this Jan 11, 2019
@aleclarson aleclarson added the bug label Jan 11, 2019
@grassick
Copy link
Author

There are two different types of currying, I see now. Only one of the two should allow the first argument to be undefined. The more complex one with a defaultBase should still take Base | undefined, I think, but not the simpler one that I'm using?

@aleclarson
Copy link
Member

Yup, I'm about to push a patch!

@aleclarson
Copy link
Member

🎉 This issue has been resolved in version 1.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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