RFC: Improvement of Typings for NGXS State Operators

[markwhitfeld]

Background
State operators are one of the incredibly handy utilities that come out of the box for NGXS.
They assist with doing more declarative, immutability safe and type-safe updates to state as well as providing a pattern as the basis for creating reusable state updates that match the particular application domain.

Problem
State Operators were introduced when Typescript could finally support the necessary type constructs through conditional typing (in TS2.8). Everything was right with the world... That is until TS3.1 landed and the Typescript typings became stricter and stricter, forcing much of the clean, composable syntax that StateOperators offered to be forced to include more explicit type in the usages of the operators. In the initial paradigm, State Operators were naturally type-safe and added minimal overhead for the safety the constraints imposed by TS3.1 added extra code with marginal benefit.
The goal of this RFC is to explore if there is a way to leverage newer constructs of Typescript to provide all the safety without the additional explicit typings code.

Context
Given the following existing types in NGXS (irrelevant parts omitted):

type StateOperator<T> = (existing: Readonly<T>) => T;

interface StateContext<T> {
  setState(val: T | StateOperator<T>): T;
}

and the following app specific code:

interface MyModel {
  foo: number;
  bar: boolean;
  baz?: string;
}
const ctx: StateContext<MyModel>; // from the first argument of an @Action function

We can perform state updates in the following way:

ctx.setState({ foo: 1, bar:'123' });
ctx.setState((state) => ({...state, foo: 234}));

All of the above mechanisms work well. One important requirement to note here is that when you delete the foo property in each case and then trigger type completion, it should suggest the foo property in the type completion.

We can also perform a state update with the patch operator:

// This works well:
ctx.setState(patch({ foo: 234 }));
// This gives a type error if you provide an invalid property:
ctx.setState(patch({ shjfadklsja: 3 })); 
// BUT this does not give an error on the invalid prop of you provide at least one valid prop:
ctx.setState(patch({ foo: 234, shjfadklsja: 3 })); 

There is also no type completion within the patch object!
This is VERY broken!!!

1. This previously worked in the initial implementation.
2. Once we were forced to have stricter type declarations (from TS3.1 upwards) then these regressions were introduced (without us knowing).
   This PR introduced the regression: fix(store): explicit typings for patch operator #1405
   It was made in response to this issue: 🐞[BUG]: State operators typing issue #1375

Also note: one of the features of State Operators is that you can roll your own operator with a function that returns a state operator.
Let's look at this code:

function customOperator<T>(partial: Partial<T>): StateOperator<T> {
  return (state) => ({ ...state, ...partial })
}
// This usage works:
ctx.setState(customOperator<MyModel>({ foo: 234 }));
// But this fails the typing if the type T is left out:
ctx.setState(customOperator({ foo: 234 }));

This is not ideal. Potentially we need to provide a helper for people creating their own operators to make this cleaner and still keep the type safety of the initial implementation.

All of the above is seeding this discussion...
How do we fix the typings of the patch operator and provide a clean way to create new ones.
We need to get this typing sorted out once and for all!

Playground

Here is a Typescript playground with some of my attempts.

[renatoalencar]

Following the discussion we had, I've been investigating this and I've come to some conclusions, not being able to tell why this happens though. First, this happens because TypeScript is inferring the type T from the parameter type not matter how general or broader that type is.

type StateOperator<T> = (args: Readonly<T>) => T

function someOperator<T>(args: Partial<T>): StateOperator<T> {
....
}

interface Model {
    a: number
    b: number
}

const operator: StateOperator<Model> = someOperator({ a: 1 })
/* It infers the parameter type and the result of the function call as
   StateOperator<{ a: number }>, which makes auto completion not
   available for the parameter value and it also doesn't typecheck
   because { a: number } is not a subtype of Model because it misses
   the b property. */

Interesting though, is that someOperator({ a: 1 } as unknown) infers StateOperator<T> to be StateOperator<unknown>, even if it could infer from the expected return type, only declaring the type of operator makes it has this type due to casting.

Using a different approach makes it typecheck, but with no auto completion working. Which is the reason why some of the operators work, but without auto completion.

function someOperator<A, T extends A>(args: Partial<A>): StateOperator<T> {
...
}
/* This works because Model is a subtype of { a: number }, which makes the condition
   T extends A being true. */

[markwhitfeld]

For reference, these are my current attempts. I have included this as a starter playground in the discussion description:

interface Model {
    foo: number
    bar: string
}

type StateOperator<T> = (state: Readonly<T>) => T

function setState<T>(operator: T | StateOperator<T>): void { }

function old_code_advanced_recursive_patching() { // WORKED FOR TS2.8 - but fails since TS3.1
    type PatchSpec<T> = { [P in keyof T]?: T[P] | StateOperator<NonNullable<T[P]>> };
    type PatchValues<T> = {
        readonly [P in keyof T]?: T[P] extends (...args: any[]) => infer R ? R : T[P];
    };

    function customOperator<T>(patchObject: PatchSpec<T>) {
        const patchValue: Partial<T> = patchObject as any; // Implementation not included
        return (state: T) => ({ ...state, ...patchObject });
    }
    /* IMPLICIT TYPES */
    // Noraml usage
    setState<Model>(customOperator({ foo: 1 })) // ## FAILED! ## - BROKEN since TS3.1
    // Should fail unknown prop
    setState<Model>(customOperator({ foo: 1, asdf: '' })) // PASS
    // Should fail all props unknown
    setState<Model>(customOperator({ asdf: '' })) // PASS
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator({ })) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator({ foo: 1, })) // ## FAILED! ## NO INTELLISENSE - BROKEN since TS3.1

    /* EXPLICIT TYPES */
    // Normal usage
    setState<Model>(customOperator<Model>({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator<Model>({ foo: 1, asdf: '' })) // PASS
    // Should fail all props unknown
    setState<Model>(customOperator<Model>({ asdf: '' })) // PASS
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator<Model>({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator<Model>({ foo: 1, })) // PASS
}

function current_code_advanced_recursive_patching() {
    type PatchSpec<T> = { [P in keyof T]?: T[P] | StateOperator<NonNullable<T[P]>> };

    type PatchValues<T> = {
        readonly [P in keyof T]?: T[P] extends (...args: any[]) => infer R ? R : T[P];
    };

    type PatchOperator<T> = <U extends PatchValues<T>>(existing: Readonly<U>) => U;

    function customOperator<T>(patchObject: PatchSpec<T>): PatchOperator<T> {
        const patchValue: Partial<T> = patchObject as any; // Implementation not included
        return ((state: T) => ({ ...state, ...patchValue })) as unknown as PatchOperator<T>;
    }
    /* IMPLICIT TYPES */
    // Noraml usage
    setState<Model>(customOperator({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator({ foo: 1, asdf: '' })) // ## FAILED! ## - Allows invalid prop! [CRITICAL BUG!]
    // Should fail all props unknown
    setState<Model>(customOperator({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator({ foo: 1, })) // ## FAILED! ## NO INTELLISENSE

    /* EXPLICIT TYPES */
    // Normal usage
    setState<Model>(customOperator<Model>({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator<Model>({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator<Model>({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator<Model>({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator<Model>({ foo: 1, })) // PASS
}

function current_code_simplified_shallow_patch_variant() {
    type PatchOperator<T> = <U extends T>(existing: Readonly<U>) => U;

    function customOperator<T>(partial: Partial<T>): PatchOperator<T> {
        return (state) => ({ ...state, ...partial })
    }
    /* IMPLICIT TYPES */
    // Noraml usage
    setState<Model>(customOperator({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator({ })) // ## FAILED! ## NO INTELLISENSE
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator({ foo: 1, })) // ## FAILED! ## NO INTELLISENSE

    /* EXPLICIT TYPES */
    // Normal usage
    setState<Model>(customOperator<Model>({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator<Model>({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator<Model>({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator<Model>({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator<Model>({ foo: 1, })) // PASS
}

function simpified_shallow_patch_attempt1() {
    function customOperator<U, T extends U = U>(partial: Partial<U>): StateOperator<T> {
        return (state) => ({ ...state, ...partial })
    }
    /* IMPLICIT TYPES */
    // Noraml usage
    setState<Model>(customOperator({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator({ })) // ## FAILED! ## NO INTELLISENSE
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator({ foo: 1, })) // ## FAILED! ## NO INTELLISENSE

    /* EXPLICIT TYPES */
    // Normal usage
    setState<Model>(customOperator<Model>({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator<Model>({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator<Model>({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator<Model>({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator<Model>({ foo: 1, })) // PASS
}

function simpified_shallow_patch_attempt2() {
    function customOperator<T>(partial: Partial<T>): StateOperator<T> {
        return (state) => ({ ...state, ...partial })
    }
    /* IMPLICIT TYPES */
    // Noraml usage
    setState<Model>(customOperator({ foo: 1 })) // ## FAILED! ##
    // Should fail unknown prop
    setState<Model>(customOperator({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator({ })) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator({ foo: 1, })) // ## FAILED! ## NO INTELLISENSE

    /* EXPLICIT TYPES */
    // Normal usage
    setState<Model>(customOperator<Model>({ foo: 1 })) // PASS
    // Should fail unknown prop
    setState<Model>(customOperator<Model>({ foo: 1, asdf: '' })) // PASS - Type error shown
    // Should fail all props unknown
    setState<Model>(customOperator<Model>({ asdf: '' })) // PASS - Type error shown
    // Test Intellisense - place cursor between the braces and try autocomplete
    setState<Model>(customOperator<Model>({})) // PASS
    // Test Intellisense - place cursor between the braces as next prop after foo and try autocomplete
    setState<Model>(customOperator<Model>({ foo: 1, })) // PASS
}

[david-shortman]

Behold! One little trick, and everything works as expected.

    // Behold! The little `extends infer S ? S : never` trick allows Typescript to infer the intended shape of T from
    // the usage of customOperator within a surrounding function.
    // I forgot why this works, but NgRx uses this trick for their `on` function in their reducers.
    function customOperator<T>(patchObject: PatchSpec<T extends infer S ? S : never>): StateOperator<T> {
        const patchValue: Partial<T> = patchObject as any; // Implementation not included
        return (state: T) => ({ ...state, ...patchValue });
    }

[david-shortman]

@markwhitfeld I would be happy to create a PR.

Also someone on the TS discord advised that this behavior comes from Contextual Inference which is documented here- https://www.typescriptlang.org/docs/handbook/type-inference.html#contextual-typing

[david-shortman]

I have some questions to author a full PR on this draft- #1806

[david-shortman]

@markwhitfeld @timdeschryver I've written up a draft tutorial on why and how NoInfer is used, and how type inference deference and contextual typing are at play. Please feel free to give any notes before I publish later this week- https://dev.to/davidshortman/weird-ts-types-how-to-plan-an-alien-abduction-using-contextual-typing-and-deferred-inference-1p49-temp-slug-8643168?preview=30e562c1a47b04648f677d3a928bec4dc784af2939503f00de17dfb3e7b32567ca3ec4468800070fdc729e6a2a4a1d8f3e3046784111d46a27104a55

[timdeschryver]

Look good @david-shortman , I like the fun example 👍

[david-shortman]

Published 😎: https://dev.to/davidshortman/weird-ts-types-using-contextual-typing-and-deferred-inference-to-plan-an-alien-conquest-bm8

Still welcome feedback if there's any to be had!