Skip to content

NonNullable is accidentally simplified away from generic index accesses #50539

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
Andarist opened this issue Aug 30, 2022 · 2 comments Β· Fixed by #50540
Closed

NonNullable is accidentally simplified away from generic index accesses #50539

Andarist opened this issue Aug 30, 2022 · 2 comments Β· Fixed by #50540
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.

Comments

@Andarist
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

defer, index access, index signature, nonnullable

πŸ•— Version & Regression Information

  • This somewhat changed between versions 4.7 and 4.8

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

interface StateSchema {
  states?: {
    [key: string]: StateSchema;
  };
}

declare class StateNode<TStateSchema extends StateSchema> {
  schema: TStateSchema;
}

// worked in 4.7 but perhaps accidentally
type StateNodesConfigCurrent<TStateSchema extends StateSchema> = {
  [K in keyof TStateSchema["states"]]: StateNode<TStateSchema["states"][K]>;
};

// this OTOH should work right now but it doesn't
type StateNodesConfigNew<TStateSchema extends StateSchema> = {
  [K in keyof TStateSchema["states"]]: StateNode<
    NonNullable<TStateSchema["states"]>[K]
  >;
};

πŸ™ Actual behavior

NonNullable disappears when computing getConstraintFromIndexedAccess. Essentially this type:

NonNullable<TStateSchema["states"]>[keyof TStateSchema["states"]]

gets simplified/distributed to

TStateSchema["states"][string] | TStateSchema["states"][number] | TStateSchema["states"][symbol]

As we might notice here - NonNullable is gone from here because of the simplification rules:

// (T | U)[K] -> T[K] | U[K] (reading)
// (T | U)[K] -> T[K] & U[K] (writing)
// (T & U)[K] -> T[K] & U[K]

The third case applies here so we end up with something like this

TStateSchema["states"][string] & {}[string] | TStateSchema["states"][number] & {}[number] | TStateSchema["states"][symbol] & {}[symbol]

In here, those {}[string | number | symbol] are just reduced to unknown and T & unknown gets reduced to T. So that's how NonNullable "disappears" here.

Note that even though T[number | symbol] is not valid for this T this indexed access is allowed based on this logic:
https://github.dev/microsoft/TypeScript/blob/488d0eebd0556fcc6e5f4cfc69c2ef7e5c2708ed/src/compiler/checker.ts#L16036-L16040

πŸ™‚ Expected behavior

NonNullable should be correctly retained/deferred in this case to preserve the correct constraint of the generic type.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 30, 2022
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.9.0 milestone Aug 30, 2022
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Aug 31, 2022
@kasparkallas
Copy link

kasparkallas commented Sep 1, 2022

After updating from 4.7 to 4.8, I believe I'm running into a related issue. Unfortunately I'm not able to explain it in such technical detail but here's a reproducible:

type Ordering<TOrderBy extends string> = {
    orderBy: TOrderBy
}

type Query<TOrderBy extends string> = {
    order?: Ordering<TOrderBy>
}

type QueryHandler<
    TQuery extends Query<TOrderBy>,
    TOrderBy extends string = NonNullable<TQuery["order"]>["orderBy"] // Worked in 4.7
> = {
    }
... does not satisfy the constraint 'string'.
... cannot be used to index type 'NonNullable...

Playground Link

Disabling strictNullChecks hides the problem.

@Andarist
Copy link
Contributor Author

Andarist commented Sep 1, 2022

I can confirm that this is also fixed by my PR. I'm gonna add this as an additional test case in a moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants