typescript-bot
commented
10 months ago Closed typescript-bot closed 10 months ago
typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago typescript-bot
commented
10 months ago 
MichaelMitchell-at
commented
10 months ago 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
commented
10 months ago @MichaelMitchell-at fix at #57113
RyanCavanaugh
commented
10 months ago 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
commented
10 months ago @MichaelMitchell-at fix at https://github.com/microsoft/TypeScript/pull/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
commented
10 months ago 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
commented
10 months ago 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
commented
10 months ago @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);
}
}
sandersn
commented
10 months ago 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
commented
10 months ago 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
commented
10 months ago Here's an independent repro for qwik's use-on errors. This one fails after https://github.com/microsoft/TypeScript/pull/56598 and passes before (where 'before' includes https://github.com/microsoft/TypeScript/pull/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);
};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
commented
10 months ago @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
commented
10 months ago It repros for me, so maybe it's the tsconfig I'm using. I'll create a standalone repo.
sandersn
commented
10 months ago Here's the standalone repo: https://github.com/sandersn/repro56598
ahejlsberg
commented
10 months ago 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
commented
10 months ago 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);
};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.
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
ts-expect-errordue to #56908, Intentional isolatedModules correctness improvement