Skip to content

Detect symbol-named properties as union discriminants #36463

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
Zemnmez opened this issue Jan 27, 2020 · 13 comments
Closed

Detect symbol-named properties as union discriminants #36463

Zemnmez opened this issue Jan 27, 2020 · 13 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@Zemnmez
Copy link

Zemnmez commented Jan 27, 2020

TypeScript Version: 3.7.5

Search Terms:
discriminated union symbol, AsyncIterator, Iterator.

Expected behavior:
Unions discriminated by symbol members should be narrowable.

Actual behavior:
No narrowing occurs.

Code

type m = {
    [Symbol.unscopables](): undefined
    cool:1
}

type b = {
    [Symbol.iterator](): undefined
    [Symbol.unscopables]():2
}

declare const k: m | b
declare function f(v: b): void


/*
Element implicitly has an 'any' type because expression of type
'symbol' can't be used to index type 'm | b'.(7053)
*/
if (k[Symbol.iterator] == undefined) f(k);


type m2 = {
  [Symbol.iterator](): 1
}

type b2 = {
  [Symbol.unscopables](): 2
}

declare function f2(b: b2): void;
declare const k2: m2 | b2;

/*
Argument of type 'm2 | b2' is not assignable to parameter of type 'b2'.
  Property '[Symbol.unscopables]' is missing in type 'm2' but required in type 'b2'.(2345)
input.ts(27, 3): '[Symbol.unscopables]' is declared here.
*/
if (Symbol.unscopables in k2) f2(k2)
Output
"use strict";
/*
Element implicitly has an 'any' type because expression of type
'symbol' can't be used to index type 'm | b'.(7053)
*/
if (k[Symbol.iterator] == undefined)
    f(k);
/*
Argument of type 'm2 | b2' is not assignable to parameter of type 'b2'.
  Property '[Symbol.unscopables]' is missing in type 'm2' but required in type 'b2'.(2345)
input.ts(27, 3): '[Symbol.unscopables]' is declared here.
*/
if (Symbol.unscopables in k2)
    f2(k2);
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "useDefineForClassFields": false,
    "alwaysStrict": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "downlevelIteration": false,
    "noEmitHelpers": false,
    "noLib": false,
    "noStrictGenericChecks": false,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "esModuleInterop": true,
    "preserveConstEnums": false,
    "removeComments": false,
    "skipLibCheck": false,
    "checkJs": false,
    "allowJs": false,
    "declaration": true,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "ES2017",
    "module": "ESNext"
  }
}

Playground Link: Provided

@Zemnmez
Copy link
Author

Zemnmez commented Jan 27, 2020

My specific usecase is to determine if a value is an AsyncIterator<T> or an Iterator<T>.

@tadhgmister
Copy link

tadhgmister commented Jan 28, 2020

first your example would be a little clearer if you just declare the variables instead of using void 0 as any as b:

declare const m: m | b;
declare function f(v: b): void;

second you don't need to use symbols to demonstrate this behaviour:

interface A {
    x: undefined;
    y: number;
}
interface B {
    x: number;
    y: number;
}
declare const val: A | B
declare function f(v: B): void

if( "x" in val) f(val);

And the reason typescript doesn't narrow the type in this case is because "field" in x still evaluates to true if the object explicitly defines the field to be undefined:

let test = {
    x: undefined,
    [Symbol.iterator]: undefined
}
console.log("x" in test); // true
console.log(Symbol.iterator in test) // true

This is why checking for just the presence of a field never narrows types in typescript since typescript is never sure what extra fields might exist at runtime.

@tadhgmister
Copy link

You probably want to use a type assertion function:

function isAsyncGen<T>(x: T): x is Extract<T, AsyncIterable<any>>{
    return (Symbol.asyncIterator in x) && x[Symbol.asyncIterator];
}
declare const x: string[] | AsyncIterable<string>

if(isAsyncGen(x)){
  // here we know x is AsyncIterable<string>
} else {
   // here we know x is string[]
}

@Zemnmez
Copy link
Author

Zemnmez commented Jan 28, 2020

hi @tadhgmister thanks for pointing that out! I set them to undefined to make the example clearer (was going to test whether the member was undefined or not...) but I messed up! I've fixed the examples now.

I'm aware that this problem can be solved with type guards, but I don't think that makes this any less of a bug!

@tadhgmister
Copy link

I don't think this is a bug, when strict null check is disabled it is totally valid for any variable to be undefined so checking if a field is undefined can't do type narrowing. Also typescript doesn't do narrowing based on just checking conditions of a field unless it is constant for union elements.

declare const val: {a: string | number};
declare function f(arg: {a: string}):void
// Argument of type '{ a: string | number; }' is not assignable to parameter of type '{ a: string; }'.
// still get error even though we know .a is a string.
if(val.a == "hello") f(val);
 
// this is allowed because val.a is narrowed.
if(val.a == "world") f({a: val.a})

// if the union is differentiated by constant values (like number literals) then union is narrowed
declare const bar: {a: string, c: 0} | {a: number, c: object}
// bar is narrowed here because it checks a field that has constant type
if(bar.c == 0) f(bar);

These are the kinds of cases where type guard functions are designed for. :)

@Duncan3142
Copy link

Duncan3142 commented Jan 30, 2020

I'm experiencing the same behaviour, intuitively this seems like a bug. The compiler should have enough information to narrow the types.

const mySym = Symbol();

type TOne = {
  [mySym]: 'one',
  val1: number
}

type TTwo = {
  [mySym]: 'two',
  val2: number
}

type TUnion = TOne | TTwo;

function getVal(a: TUnion): number {
  switch (a[mySym]) {
    case 'one': {
      return a.val1 // Error
    }
    case 'two': {
      return a.val2 // Error
    }
  }
}

Playground Link

@tadhgmister
Copy link

Oh ok now I see the issue. I do notice that if you change it to mySym = "hello" then a["hello"] does do narrowing but a[mySym] does not even though it is known to be the constant string, or in your case a constant symbol. Sorry for not getting it initially 🤷‍♂

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Feb 6, 2020
@RyanCavanaugh RyanCavanaugh changed the title discriminated unions on symbol members are not narrowed Detect symbol-named properties as union discriminants Feb 6, 2020
@boneskull
Copy link
Contributor

boneskull commented Feb 20, 2020

I think I'm experiencing this issue (using JS).

Something like this trivial example:

const kName = Symbol('name');

class Frog {
  /** 
   * @param {string} name - Name of frog
   */
  constructor(name) {
    this[kName] = name; // Element implicitly has 'any' type because expression of type 'symbol' can't be used to index type 'Frog'
  }

  get name() {
    return this[kName]; // Element implicitly has 'any' type because expression of type 'symbol' can't be used to index type 'Frog'
  }
}

A workaround is something horrid like:

  /** @type {string} */
  get name() {
    return Reflect.get(this, kName);
  }

@tadhgmister
Copy link

tadhgmister commented Feb 20, 2020

I think I'm experiencing this issue (using JS).

Something like this trivial example:

let kName = Symbol('name');

class Frog {
  /** 
   * @param {string} name - Name of frog
   */
  constructor(name) {
    this[kFoo] = name; // Element implicitly has 'any' type because expression of type 'symbol' can't be used to index type 'Frog'
  }

  get name() {
    return this[kName]; // Element implicitly has 'any' type because expression of type 'symbol' can't be used to index type 'Frog'
  }
}

try using kName or kFoo consistently since you use both, also use const for it so typescript knows it won't get reassigned so it can treat it as a unique symbol. Playground Link

const kFoo = Symbol('name');

class Frog {
  /** 
   * @param {string} name - Name of frog
   */
  constructor(name) {
    this[kFoo] = name;
  }

  get name() {
    return this[kFoo]; 
  }
}

@tadhgmister
Copy link

I think this suggestion is a specific case of #36230. If we let constant property access narrow types then it would automatically extend to allow symbols for discrimination.

@boneskull
Copy link
Contributor

@tadhgmister Sorry, I was inconsistent in that example. I've updated it to reflect the actual code. It is a problem either way

@tadhgmister
Copy link

@boneskull You are still experiencing a different problem, if the error says 'any' type because expression of type 'symbol' then it means typescript isn't detecting that you have a unique symbol.

use const for it so typescript knows it won't get reassigned so it can treat it as a unique symbol. Playground Link

This is the same issue with having let field = "fieldName" can't detect that x[field] is equivalent to x.fieldName because it is type string because it may change at runtime. Using const fixes it for both strings and symbols.

@Zemnmez
Copy link
Author

Zemnmez commented Apr 12, 2022

This was fixed in #36230

@Zemnmez Zemnmez closed this as completed Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants