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

Inference inconsistency with string keys #26257

Closed
yortus opened this issue Aug 7, 2018 · 5 comments
Closed

Inference inconsistency with string keys #26257

yortus opened this issue Aug 7, 2018 · 5 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@yortus
Copy link
Contributor

yortus commented Aug 7, 2018

TypeScript Version: 3.1.0-dev.20180807

Search Terms:
string index inference, string key inference

Code

const SYM = Symbol();
type StringProps1<T> = T extends {[x: string]: infer U} ? U : never;
type StringProps2<T> = T extends {[K in Extract<keyof T, string>]: infer U} ? U : never;

type T1 = StringProps1<{a: 'aaa', [SYM]: 'sym'}>;   // T1 = 'aaa' | 'sym'
type T2 = StringProps2<{a: 'aaa', [SYM]: 'sym'}>;   // T2 = 'aaa'

Expected behavior:
I expected StringProps1 and StringProps2 to mean the same thing (infer from just the string keys).

Accordingly, T1 and T2 would both inferred as 'aaa', both excluding the symbol key.

Actual behavior:

T2 excludes the symbol key as expected, but T1 includes it, as shown in the code comments.

Even if StringProps1 and StringProps2 did have subtly different meanings, it doesn't seem logical to include the [SYM]: 'sym' as part of the string indexer, since it doesn't belong there.

Related Issues:

This is a simpler repro of #26183, although that issue shows its nothing specific to conditional types. It seems to be just how string indexers work with type inference.

The search terms above brought up #25065 and #24560, but they don't look like the same issue.

@ghost ghost added Bug A bug in TypeScript Domain: Mapped Types The issue relates to mapped types and removed Domain: Mapped Types The issue relates to mapped types labels Aug 7, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.1 milestone Aug 8, 2018
@ahejlsberg
Copy link
Member

This is working as intended. A string index signature places a constraint on every property of a type, regardless of what kind of name it has. I agree the name "string index signature" is a bit unfortunate as it affects properties that might have numeric or symbol names, but the name predates our support for those kinds of names. It would definitely be a breaking change to modify our behavior here.

@ahejlsberg ahejlsberg removed their assignment Aug 9, 2018
@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Aug 9, 2018
@ahejlsberg ahejlsberg removed this from the TypeScript 3.1 milestone Aug 9, 2018
@yortus
Copy link
Contributor Author

yortus commented Aug 10, 2018

Oh right. I just assumed the strings and symbols were separated out as part of #23592 (and the breaking change taken there).

So what about this third variation, is it also working as intended?

const SYM = Symbol();
type StringProps3<T> = T extends {[K in string]: infer U} ? U : never;
type T3 = StringProps3<{a: 'aaa', [SYM]: 'sym'}>;   // T2 = 'aaa' | 'sym'

The K in that mapped type is also picking up the symbol key, so K in string is including non-string values.

@yortus
Copy link
Contributor Author

yortus commented Aug 10, 2018

Another thing I can't figure out is, if {[x: string]: T} is intended to apply to all keys (i.e., strings, symbols and numbers), then why is the following not an error?

const SYM = Symbol();
declare function foo(obj: {[x: string]: number}): void;
foo({a: 1, [SYM]: 'sym'});  // OK

In this case, the symbol key appears not to be part of the 'string index signature' since it is not constrained to number.

From my perspective, this behaviour seems correct (ie symbols are excluded from the string index signature), and the prior 'working as intended' behaviour seems incorrect. But either way, shouldn't the inferring and the constraining behaviour both be consistent w.r.t. including symbols in the string index signature?

@yortus
Copy link
Contributor Author

yortus commented Aug 10, 2018

For context, the following code is a cut-down repro of what I'm trying to do, but I can't figure out what's going on with the compiler. Playground link.

The intent here is that all three cases should work without errors. According to @ahejlsberg's comment above I suspect all three should be errors. In reality 2/3 work. In the end, is there a way to express the intended inference logic of the foo function?

// function foo takes an 'options' object that can have:
// (a) any number of string keys whose values are unary functions
// (b) an optional 'directive' unique symbol key with a unrelated type
const directive = Symbol('directive');
declare function foo<TArg, TRet, TDir>(options:
    {[x in string]: (arg: TArg) => TRet}    // all the string keys are unary functions
    & {[directive]?: TDir}                  // there is an optional 'directive' symbol key
): void;

// CASE 1
// ~~~ERROR~~~ - ... prop 'addOne' incompatible with index signature
//               ... 'x' and 'arg' are incompatible
//               ... 'string' not assignable to 'number'
let case1 = foo({
    [directive]: (x: string) => 'str',
    addOne: (x: number) => x + 1,
    double: (x: number) => x + x,
});

// CASE 2: identical to case 1 except for order of properties
// Compiles OK - infers TArg = number, TRet = number, TDir = (x: string) => string
let case2 = foo({
    addOne: (x: number) => x + 1,
    double: (x: number) => x + x,
    [directive]: (x: string) => 'str',
});


// CASE 3: identical to case 1 except for type of directive
// Compiles OK - infers TArg = number, TRet = number, TDir = string
let case3 = foo({
    [directive]: 'str',
    addOne: (x: number) => x + 1,
    double: (x: number) => x + x,
});

@yortus
Copy link
Contributor Author

yortus commented Aug 15, 2018

Closing since it's working as intended. I opened a separate issue (#26470) for the code in the preceding comment.

@yortus yortus closed this as completed Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants