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

[NewErrors] 5.4.0-dev.20240121 vs 5.3.3 #57117

Closed
typescript-bot opened this issue Jan 21, 2024 · 29 comments
Closed

[NewErrors] 5.4.0-dev.20240121 vs 5.3.3 #57117

typescript-bot opened this issue Jan 21, 2024 · 29 comments

Comments

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 21, 2024

The following errors were reported by 5.4.0-dev.20240121, but not by 5.3.3
Pipeline that generated this bug
Logs for the pipeline run
File that generated the pipeline

This run considered 200 popular TS repos from GH (after skipping the top 0).

Successfully analyzed 115 of 200 visited repos
Outcome Count
Detected interesting changes 12
Detected no interesting changes 103
Git clone failed 3
Package install failed 35
Project-graph error in old TS 5
Too many errors in old TS 38
Unknown failure 4

Investigation Status

Repo Errors Outcome
BuilderIO/qwik 2 #56004, #56598
chakra-ui/chakra-ui 1 Not a regression
drizzle-team/drizzle-orm 16 #56004
heyxyz/hey 1 Intentional isolatedModules correctness improvement
pixijs/pixijs 2 Routine DOM update #57027
prisma/prisma 10 Intentional isolatedModules correctness improvement
pubkey/rxdb 3 #54477, only showed up in editor for some reason
quilljs/quill 3 Unused ts-expect-error due to #56908, Intentional isolatedModules correctness improvement
react-hook-form/react-hook-form 1 #56004
reduxjs/react-redux 2 #56515
refined-github/refined-github 5 #56004
vuejs/core 3 #55015, #56004
@typescript-bot
Copy link
Collaborator Author

BuilderIO/qwik

10 of 11 projects failed to build with the old tsc and were ignored

tsconfig.json

@typescript-bot
Copy link
Collaborator Author

chakra-ui/chakra-ui

4 of 28 projects failed to build with the old tsc and were ignored

packages/components/tsconfig.build.json

  • error TS5056: Cannot write file '/mnt/ts_downloads/chakra-ui/packages/components/dist/types/menu/menu.stories.d.ts' because it would be overwritten by multiple input files.
    • Project Scope

@typescript-bot
Copy link
Collaborator Author

drizzle-team/drizzle-orm

18 of 25 projects failed to build with the old tsc and were ignored

drizzle-orm/tests/tsconfig.json

  • error TS2345: Argument of type 'SQLiteD1Session<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2345: Argument of type 'LibSQLSession<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'TFullSchema extends Record<string, never> ? DrizzleTypeError<"Seems like the schema generic is missing - did you forget to add it to your DB type?"> : { [K in keyof TSchema]: RelationalQueryBuilder<TResultKind, TFullSchema, TSchema, TSchema[K]>; }'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'this["query"]'.

drizzle-orm/tsconfig.dts.json

  • error TS2345: Argument of type 'SQLiteD1Session<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2345: Argument of type 'LibSQLSession<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'TFullSchema extends Record<string, never> ? DrizzleTypeError<"Seems like the schema generic is missing - did you forget to add it to your DB type?"> : { [K in keyof TSchema]: RelationalQueryBuilder<TResultKind, TFullSchema, TSchema, TSchema[K]>; }'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'this["query"]'.

drizzle-orm/tsconfig.json

  • error TS2345: Argument of type 'SQLiteD1Session<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2345: Argument of type 'LibSQLSession<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'TFullSchema extends Record<string, never> ? DrizzleTypeError<"Seems like the schema generic is missing - did you forget to add it to your DB type?"> : { [K in keyof TSchema]: RelationalQueryBuilder<TResultKind, TFullSchema, TSchema, TSchema[K]>; }'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'this["query"]'.

drizzle-orm/type-tests/tsconfig.json

  • error TS2345: Argument of type 'SQLiteD1Session<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2345: Argument of type 'LibSQLSession<TSchema, ExtractTablesWithRelations<TSchema>>' is not assignable to parameter of type 'SQLiteSession<"async", unknown, Record<string, unknown>, TablesRelationalConfig>'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'TFullSchema extends Record<string, never> ? DrizzleTypeError<"Seems like the schema generic is missing - did you forget to add it to your DB type?"> : { [K in keyof TSchema]: RelationalQueryBuilder<TResultKind, TFullSchema, TSchema, TSchema[K]>; }'.
  • error TS2536: Type 'keyof TSchema' cannot be used to index type 'this["query"]'.

@typescript-bot
Copy link
Collaborator Author

heyxyz/hey

1 of 12 projects failed to build with the old tsc and were ignored

apps/web/tsconfig.json

@typescript-bot
Copy link
Collaborator Author

pixijs/pixijs

tsconfig.json

tsconfig.types.json

@typescript-bot
Copy link
Collaborator Author

prisma/prisma

75 of 106 projects failed to build with the old tsc and were ignored

packages/debug/tsconfig.build.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/debug/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/get-platform/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/engines/tsconfig.build.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/engines/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/generator-helper/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/internals/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/instrumentation/tsconfig.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/client/tsconfig.build.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

packages/cli/tsconfig.build.json

  • error TS2865: Import 'Debug' conflicts with local value, so must be declared with a type-only import when 'isolatedModules' is enabled.

@typescript-bot
Copy link
Collaborator Author

pubkey/rxdb

9 of 11 projects failed to build with the old tsc and were ignored

tsconfig.json

@typescript-bot
Copy link
Collaborator Author

react-hook-form/react-hook-form

2 of 3 projects failed to build with the old tsc and were ignored

tsconfig.json

@typescript-bot
Copy link
Collaborator Author

reduxjs/react-redux

2 of 3 projects failed to build with the old tsc and were ignored

test/typetests/tsconfig.json

@typescript-bot
Copy link
Collaborator Author

refined-github/refined-github

tsconfig.json

@typescript-bot
Copy link
Collaborator Author

vuejs/core

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.build.json

tsconfig.json

@MichaelMitchell-at
Copy link

Copying over my comment from #56972

Minimal repro of a "cannot be used as an index type" error:

type Type = {prop: 'a', otherProp: unknown}

type X = 'prop' extends keyof Type
    ? Type['prop']
           ^^^^^^ Type '"prop"' cannot be used as an index type.(2538)
    : never;

@RyanCavanaugh
Copy link
Member

@MichaelMitchell-at fix at #57113

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 23, 2024

react-redux is trying to do the should-be-impossible and prevent function arguments from subtyping with this function

export function expectExactType<T>(t: T) {
  return <U extends Equals<T, U>>(u: U) => {}
}

demonstrated here:

// @ts-expect-error
expectExactType('5' as string)('5' as const)

and also to prevent any from bypassing errors

// @ts-expect-error
expectExactType('5' as any)('5' as const)

To the extent they once succeeded, they're now broken by #56515. However, this functionality was never used AFAICT, every call is in the form e.g.

expectExactType<AppStore>(store)

so I don't think this is a blocker for that project in particular.

@Andarist
Copy link
Contributor

Andarist commented Jan 23, 2024

@MichaelMitchell-at fix at #57113

It's worth noting that this is a fix for the @MichaelMitchell-at's minimal repro case but that minimal repro case likely isn't related to Drizzle's "cannot be used as an index type" problems (I know you have already bisected this to the PR that caused this particular change). Unless I'm mistaken, we only have changes between 5.3 and 5.4 reported here (the title of the issue suggests so :P) - and that minimal repro case already doesn't typecheck with 5.3

@jakebailey
Copy link
Member

Unless I'm mistaken, we only have changes between 5.4 and 5.4 reported here (the title of the issue suggests so :P) - and that minimal repro case already doesn't typecheck with 5.3

This PR compares errors between 5.3.3 and 5.4.0-dev.20240121 (not 5.4 and 5.4); but if it didn't build with 5.3, common errors will be ignored.

@Andarist
Copy link
Contributor

That was just a typo - I meant what you have confirmed 😉 That minimal repro didn't typecheck with 5.3 so it just continues to be an error with the current nightly of 5.4 and because of that it's unlikely to be related to any errors reported here.

@andrewbranch
Copy link
Member

@ahejlsberg react-hook-form repro:

type AsyncDefaultValues<TFieldValues> = (
  payload?: unknown,
) => Promise<TFieldValues>;

type DefaultValues<TFieldValues> =
  TFieldValues extends AsyncDefaultValues<TFieldValues>
    ? Awaited<TFieldValues>
    : TFieldValues;

function f<T extends Record<string, any>>(options: DefaultValues<T> | AsyncDefaultValues<T>) {
  if (typeof options === 'function') {
    options().then((data) => data);
  }
}

https://www.typescriptlang.org/play?module=1&ts=5.4.0-dev.20240124#code/C4TwDgpgBAggziAdgYwCIQGYEMCuAbYANSzxwjgB4AVAMQEsI8ATY08gPigF4oAKAKChQwWEHgD2WJgH4AXFByIA1onEB3RABp+ASm6cACgCdxAWzpwI1eoxYkycdgG5+-UJCjps+IvfLWGZlYHTi5BKFpAuzY4KAgAD2AIRCZY+CQ0TFwCYP9I21zHcKFpWDUsOiSmAIK-IqEheXygupd+DEVkYDpxRCgMajjE5NSoACUIZHEjarhgIzpEAHNNKCxEEHZ2XnEwbt64eS9s3xjqTgAfWAQUY59C870Ab3C6DD53CHF33f3EWK4gKgAHIOig-sDnsUoL8ev9eDoAHTAAAWyV4vCYWGAWD0XE4WJxOhcQgAvvxyUA

sandersn added a commit to sandersn/TypeScript that referenced this issue Jan 24, 2024
Discovered in microsoft#57117

The implementation should not `couldContainTypeVariables`--it's not
intended a fast path, and should not be used in places where its
unreliability can be observed.

The tests stay, but with a note added that they should pass but do not.
@sandersn
Copy link
Member

I double checked qwik. The errors below are definitely caused by #56598. I manually reverted that PR's checker diff and the errors went away. I'll try to make a small repro.

error TS2345: Argument of type 'EventQRL<T>' is not assignable to parameter of type 'EventQRL<"copy" | "progress" | "reset" | "scroll" | "resize" | "drag" | "fullscreenchange" | "fullscreenerror" | "abort" | "animationcancel" | "animationend" | "animationiteration" | ... 112 more ... | "qvisible">'.

@ahejlsberg
Copy link
Member

Regarding the new react-hook-form error, the issue is the Record<string, any> constraint on T. We have rule that says an object with an [x: string]: any index signature is a supertype of any object type, even if that object type doesn't have a matching index signature. This includes function types, so in the example, the typeof options === "function" check can't just narrow the type to AsyncDefaultValues<T>. Indeed, options could be any arbitrary function. To wit, this call is permitted:

f(() => "hello");

The reason we issue an implicit any error on the data parameter is that, since the function check can really only narrow to type Function, we end up with an untyped function call.

Things work fine when the constraint of T is changed to Record<string, unknown>. In that case we error on calls to f with arbitrary function types, and we narrow to AsyncDefaultValues<T> following the function check.

Other than adding a type assertion, I'm not sure what the best fix is in the react-hook-form project since new errors pop up if I just change the constraint to Record<string, unknown>. But, certainly, I think our is correct and points out a latent issue in the code.

@sandersn
Copy link
Member

sandersn commented Jan 25, 2024

Here's an independent repro for qwik's use-on errors. This one fails after #56598 and passes before (where 'before' includes #56004 -- though both are required for the error to appear.)

(I'll have a much smaller repro soon; this is just the first one that is independent of qwik)

export type Q<TYPE = unknown> = {
  // Special type brand to let eslint that the Type is serializable
  __qwik_serializable__?: any;
  __brand__Q__: TYPE;

  /** Resolve the Q and return the actual value. */
  resolve(): Promise<TYPE>;
  /** The resolved value, once `resolve()` returns. */
  resolved: undefined | TYPE;

  getCaptured(): unknown[] | null;
  getSymbol(): string;
  getHash(): string;
}
type Handler<EV = Event, EL = {}> = {
  bivarianceHack(event: EV, element: EL): any;
}['bivarianceHack'];
type VirtualKeys = keyof WMap
interface E { }
interface WMap {
    "boo": E;
}
type AllEventsMap = WMap
type LcEvent<T extends string, C extends string = Lowercase<T>> = C extends keyof AllEventsMap
  ? AllEventsMap[C]
  : Event;
type VirtualEvent<T extends string = VirtualKeys> =
  | Q<Handler<LcEvent<T>, {}>>
  | undefined;
type Stringy<LiteralType, BaseType extends null | undefined | string | number | boolean | symbol | bigint> =
  | LiteralType
  | (BaseType & Record<never, never>);
type VirtualEventNames = Stringy<VirtualKeys, string>; // VirtualKeys | (string & { });
declare const _virtualOn: (eventQrl: VirtualEvent<VirtualKeys>) => void;
export const virtualOn = <T extends VirtualEventNames>(eventQrl: VirtualEvent<T>) => {
  _virtualOn(eventQrl); // error here where none was before
};

Smaller:

// interesting things:
// 1. only errors if event in EMap is all lowercase.
// 2. bivariance hack is required
// 3. Lowercase<T> is required
type EMap = { event: {} }
type Keys = keyof EMap
type EPlusFallback<C> = C extends Keys
  ? EMap[C]
  : "unrecognised event";
type VirtualEvent<T extends string> = { bivarianceHack(event: EPlusFallback<Lowercase<T>>): any; }['bivarianceHack'];
type KeysPlusString = Keys | (string & Record<never, never>);
declare const _virtualOn: (eventQrl: VirtualEvent<Keys>) => void;
export const virtualOn = <T extends KeysPlusString>(eventQrl: VirtualEvent<T>) => {
  _virtualOn(eventQrl);
};

@sandersn
Copy link
Member

The error is the one @RyanCavanaugh and @ahejlsberg found before--KeysPlusString is correctly not assignable to Keys in VIrtualEvent.

The error seems to show that the conditional type EPlusFallback isn't assignable to its base contraint:

Argument of type '(event: EPlusFallback<Lowercase<T>>) => any' is not assignable to parameter of type '(event: {}) => any'.
  Types of parameters 'event' and 'event' are incompatible.
    Type '{}' is not assignable to type 'EPlusFallback<Lowercase<T>>'.ts(2345)

EPlusFallback<Lowercase<T>> should have the base constraint {} | "unrecognised event", but I guess that assignability doesn't try that.

Also: I guess assignability to Lowercase<T> must go through isValidTypeForTemplateLiteralPlaceholder, because that's the only thing that looks relevant to #56598.

@ahejlsberg
Copy link
Member

@sandersn I tried reverting #56598 and it doesn't seem to have any effect. Your repro still errors in the same way. So I think the issue is just caused by #56004.

@sandersn
Copy link
Member

It repros for me, so maybe it's the tsconfig I'm using. I'll create a standalone repo.

Example run of before/after showing no error/error
✔ ~/ts [main|⚑ 1] 
14:30 $ git checkout e5513254a3
Note: switching to 'e5513254a3'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at e5513254a3 Fixed symbol lookup for binding expando properties in blocks (#56552)
✔ ~/ts [:e5513254a3|⚑ 1] 
14:31 $ b
Using ~/ts/Herebyfile.mjs to run tsc
Starting lib
Starting generate-diagnostics
> /home/nathansa/.nvm/versions/node/v20.11.0/bin/node scripts/processDiagnosticMessages.mjs src/compiler/diagnosticMessages.json
Reading diagnostics from src/compiler/diagnosticMessages.json
Finished lib in 33ms
Finished generate-diagnostics in 63ms
Starting bundle-tsc
Finished bundle-tsc in 179ms
Completed tsc in 242ms
✔ ~/ts [:e5513254a3|⚑ 1] 
14:31 $ pushd
~/src/test ~/ts
t✔ ~/src/test [master L|✚ 15…16] 
14:31 $ tsl
✔ ~/src/test [master L|✚ 15…16] 
14:31 $ pushd
~/ts ~/src/test
✔ ~/ts [:e5513254a3|⚑ 1] 
14:31 $ git checkout 68b9b07
Previous HEAD position was e5513254a3 Fixed symbol lookup for binding expando properties in blocks (#56552)
HEAD is now at 68b9b07264 Consistently check assignability to template literal placeholders (#56598)
✔ ~/ts [:68b9b07264|⚑ 1] 
14:31 $ b
Using ~/ts/Herebyfile.mjs to run tsc
Starting lib
Starting generate-diagnostics
> /home/nathansa/.nvm/versions/node/v20.11.0/bin/node scripts/processDiagnosticMessages.mjs src/compiler/diagnosticMessages.json
Reading diagnostics from src/compiler/diagnosticMessages.json
Finished lib in 32ms
Finished generate-diagnostics in 61ms
Starting bundle-tsc
puFinished bundle-tsc in 190ms
Completed tsc in 252ms
s✔ ~/ts [:68b9b07264|⚑ 1] 
14:31 $ pushd
~/src/test ~/ts
✔ ~/src/test [master L|✚ 15…16] 
14:31 $ tsl
welove.ts:22:14 - error TS2345: Argument of type '(event: EPlusFallback<Lowercase<T>>) => any' is not assignable to parameter of type '(event: {}) => any'.
  Types of parameters 'event' and 'event' are incompatible.
    Type '{}' is not assignable to type 'EPlusFallback<Lowercase<T>>'.

22   _virtualOn(eventQrl);
                ~~~~~~~~


Found 1 error in welove.ts:22

✘-2 ~/src/test [master L|✚ 15…16] 

@sandersn
Copy link
Member

Here's the standalone repo: https://github.com/sandersn/repro56598

@ahejlsberg
Copy link
Member

Ok, I see the same effects with your repro. The root cause is #56004, but the issue only shows up after #56598 because that PR fixes template literal placeholder matching such that `${string & Record<never, never>}` behaves the same as just `${string}` (which indeed it should). And apparently we use template literal matching when checking relations for two string mapping types (this happens in isMemberOfStringMapping). Crazy.

@ahejlsberg
Copy link
Member

Anyway, this means the repro can be reduced to:

type EMap = { event: {} }
type Keys = keyof EMap
type EPlusFallback<C> = C extends Keys ? EMap[C] : "unrecognised event";
type VirtualEvent<T extends string> = { bivarianceHack(event: EPlusFallback<Lowercase<T>>): any; }['bivarianceHack'];
declare const _virtualOn: (eventQrl: VirtualEvent<Keys>) => void;
export const virtualOn = <T extends string>(eventQrl: VirtualEvent<T>) => {
  _virtualOn(eventQrl);
};

@ahejlsberg
Copy link
Member

Ok, I now see what's going on.

The repro checks whether EPlusFallback<Lowercase<T>> is assignable to EPlusFallback<Keys> (which is just {}).

With #56004, the constraint of EPlusFallback<Lowercase<T>> ends up correctly being type EMap["event" & Lowercase<T>] | "unrecognized event", where previously it would just be "unrecognized event".

The constraint of EMap["event" & Lowercase<T>] in turn is reduced to EMap["event" & Lowercase<string>].

This is where things go wrong. We currently lack the ability to reduce unions and intersections of string literal types (like "event") and string mapping types with non-generic placeholders (like Lowercase<string>). The intersection "event" & Lowercase<string> really should reduce to just "event", but it doesn't. So, we fail to consider it a valid index type in EMap["event" & Lowercase<string>], which therefore ends up being unknown. And unknown isn't assignable to {}, so therefore error.

So, long story short, we can fix this issue by doing a better job reducing unions and intersections of string literal types and string mapping types with non-generic placeholders. I will look at putting up a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants