Skip to content

how generic type and type inference works changed #33278

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

Closed
gogoyqj opened this issue Sep 6, 2019 · 8 comments
Closed

how generic type and type inference works changed #33278

gogoyqj opened this issue Sep 6, 2019 · 8 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@gogoyqj
Copy link

gogoyqj commented Sep 6, 2019

TypeScript Version: 3.6.3+

Search Terms:

Code
(https://github.com/gogoyqj/tkit/blob/master/packages/model/__tests__/createModelSamples/testPutCallInModelOK.ts)

createModel
[model example]

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.
export interface Reducers<M> {
  [doSomething: string]:
    | (<P extends never>(state: Readonly<M>, action: P) => M)
    | (<P extends AbstractAction>(state: Readonly<M>, action: P) => M);
}
export default function createModel<M, R extends Reducers<M>, E extends Effects>(model: {
  namespace: string;
  state: M;
  reducers: R;
  effects?: E;
}) {
...
}

const model = createModel({
  namespace: 'model',
  state: modelWithEffectsState,
  reducers: {
    doSomething: (state, action: { id: number }) => ({ ...state })
  },
  effects: {
  }
});

const { actions } = model;
actions.doSomething({ id: 1 });

Expected behavior:

from 3.3.+ to 3.5.,

while invoking createModel, the pass-in object's properties state's type determines reducers['doSomething'] first argument's type and its type is the type of modelWithEffectsState,

and reducers['doSomething'] second argument's type is the type of model.actions.doSomething's argument type - really wonderful features by generic and type inference

Actual behavior:

while since 3.6.3 reducers['doSomething'] first argument's type becomes any,

is there any way to recover these features, or changes i can make to make createModel works again?

help needed

Playground Link:

Related Issues:

@gogoyqj gogoyqj changed the title how function generic type works changed how generic type and type inference works changed Sep 6, 2019
@gogoyqj
Copy link
Author

gogoyqj commented Sep 6, 2019

and in simpler scenario it works ok

function caller2<
  M,
  P extends {
    [doSomething: string]: <P extends any>(state: Readonly<M>, action: P) => M;
  }
>(o: { state: M; reducers: P }) {
  return o;
}

caller2({
  state: { name: '2' },
  reducers: {
    '2': state => ({ ...state, name: '' })
  }
});

and little complex fail

function caller2<
  M,
  P extends {
    [doSomething: string]: 
    | (<P extends never>(state: Readonly<M>, action: P) => M)
    | (<P extends{}>(state: Readonly<M>, action: P) => M);
  }
>(o: { state: M; reducers: P }) {
  return o;
}

caller2({
  state: { name: '2' },
  reducers: {
    '2': state => ({ ...state, name: '' })
  }
});

@sandersn
Copy link
Member

sandersn commented Sep 6, 2019

@gogoyqj This might be fixed in 3.6.3. Can you try typescript@next?

@sandersn sandersn added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Sep 6, 2019
@sandersn sandersn added this to the TypeScript 3.7.0 milestone Sep 6, 2019
@gogoyqj
Copy link
Author

gogoyqj commented Sep 8, 2019

@sandersn,thanks for ur suggestion
i've tried "typescript": "^3.7.0-dev.20190907", but the problem still exists.

and i change the generic R's definition

from:
export interface Reducers<M> {
  [doSomething: string]:
    | (<P extends never>(state: Readonly<M>, action: P) => M)
    | (<P extends AbstractAction>(state: Readonly<M>, action: P) => M);
}
to:
export interface Reducers<M> {
  [doSomething: string]:
      (<P extends AbstractAction>(state: Readonly<M>, action: P) => M);
}

then it works - so does typescript version 3.6.2

@weswigham
Copy link
Member

weswigham commented Sep 11, 2019

Yeah, I can confirm that for

type Effects = {};
type AbstractAction = {};


export interface Reducers<M> {
    [doSomething: string]:
    | (<P extends never>(state: Readonly<M>, action: P) => M)
    | (<P extends AbstractAction>(state: Readonly<M>, action: P) => M);
}
export default function createModel<M, R extends Reducers<M>, E extends Effects>(model: {
    namespace: string;
    state: M;
    reducers: R;
    effects?: E;
}) {

}

const modelWithEffectsState = {};

const model = createModel({
    namespace: 'model',
    state: modelWithEffectsState,
    reducers: {
        doSomething: (state, action: { id: number }) => ({ ...state })
    },
    effects: {
    }
});

in 3.5, state is inferred as Readonly<{}>, while now it's any.

@weswigham
Copy link
Member

Our changes to compareSignaturesIdentical cause the signatures to no longer merge in contextual typing (after all, they have distinct, unique type parameters), which then cause state to not have a contextual type applied. This is maybe a little bit too strict for contextual typing, since the type parameters are going to (sometimes, like in this case) become instantiated anyway.

@weswigham
Copy link
Member

Regressed by #32049, needs a fix like #31029, but to compareSignaturesIdentical, rather than signatureRelatedTo.

@weswigham
Copy link
Member

Hmmm, so sadly I think I can only say that this really only worked essentially because of the bug fixed in #32049 - the two signatures really aren't identical - even if we try to do better and produce inferences for P before checking, since one of the constraints is never, the annotation isn't assignable to that one in that case, P becomes never in that signature, and we see that (state: Readonly<{}>, action: never) => {} isn't the same as (state: Readonly<{}>, action: { id: number }) => {}, and today we only make a contextual signature if the component signatures are identical, so... eck. I imagine that's why one of the signatures is has a P extends never rather than just a never, to create the erroneous "identical" behavior, which no longer occurs.

@gogoyqj
Copy link
Author

gogoyqj commented Sep 12, 2019

seems in early version[ 3.3?]

doSomething: state => ({ ...state })

is not compatible with:

doSomething: (<P extends AbstractAction>(state: Readonly<M>, action: P) => M)

but compatible with:

doSomething: (<P extends never>(state: Readonly<M>, action: P) => M)

that's why i using union

| (<P extends never>(state: Readonly<M>, action: P) => M)
| (<P extends AbstractAction>(state: Readonly<M>, action: P) => M)

and since version 3.4, 3.5, 3.6 are both compatible - but 3.6 works little differently

honestly maybe it's a bravo improvement not a bug since version 3.4 and i can simplify my code

@RyanCavanaugh RyanCavanaugh added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Jan 24, 2020
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.8.1 milestone Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants