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

Type narrowing not working for unions of tuples with object literals #31613

Open
squidfunk opened this issue May 28, 2019 · 11 comments
Open

Type narrowing not working for unions of tuples with object literals #31613

squidfunk opened this issue May 28, 2019 · 11 comments
Labels
Bug A bug in TypeScript Domain: Control Flow The issue relates to control flow analysis Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented.
Milestone

Comments

@squidfunk
Copy link

It's not possible to narrow unions of tuples with object literals

TypeScript Version: 3.4.5, 3.5.0-rc

Search Terms: tuple narrow object literals

Code:

Narrowing tuples with primitive types works as expected:

type X = ["a", "c"] | ["b", "b"]

function bar(x: X) {
  if (x[0] === "b") {
    return x[1] === "c" // correctly narrows to "c", shows "condition always false"
  }
}

However, when the tuple contains object literals, narrowing doesn't work at all:

interface A { data: "a" }
interface B { data: "b" }
interface C { data: "c" }

type Y = [A, C] | [B, B]

function foo(y: Y) {
  if (y[0].data === "b") {
    return y[1].data === "c" // incorrectly narrows to "c" | "b"
  }
}

Expected behavior:

The object literal within the tuple is correctly narrowed down to type B

Actual behavior:

The object literal is not narrowed, leaving it at type C | B.

Playground Link: Link

Related Issues:
#26323
#24120
#10530

@squidfunk squidfunk changed the title Type narrowing for unions of tuple elements with object literals Type narrowing not working for unions of tuples with object literals May 28, 2019
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 28, 2019
@midzdotdev
Copy link

Hi guys, not sure if this helps but here's another example which I believe to be the same issue:

type FieldArray = string[]
type FieldSet = Record<string, any>

type FieldParams =
  | [ FieldArray, FieldSet ]
  | [ FieldArray ]
  | [ FieldSet ]

function convertFieldParams (fields: FieldParams): Field[] {
  const normalised = typeof fields[1] === 'undefined'
    ? fields[0] instanceof Array
      ? [ fields[0], {} ]
      : [ [], fields[0] ]
    : fields

  // ...
}

I would expect the normalised variable to have a type of [ FieldArray, FieldSet ] due to the type checking, although it's showing in my editor as Record<string, any>[].

I found a workaround by doing this:

const normalised: [ FieldArray, FieldSet ] = 
    fields.length === 1
      ? fields[0] instanceof Array
        ? [ fields[0], {} ]
        : [ [], fields[0] ]
      : fields

@squidfunk
Copy link
Author

Any update? Fixing this issue would make tuples much more powerful :-)

@GeeWee
Copy link

GeeWee commented Sep 3, 2019

Probably same as #12849
I've narrowed the issue down a little bit, as to where it breaks down:
playground link

type DataFetchingResponse2 = [number, 'loaded'] | [null, 'error' | 'loading'];


function test2(): DataFetchingResponse2 {
	return [null, 'error'];
}

function testTypeNarrowing() {
	const res2 = test2();

    // This works fine and correctly narrows res2[0] down to number
	if (res2[1] == 'loaded') {
		const data2 = res2[0];
	}

    
    // This does not, but casts it as a number|null
    const status = res2[1]
	if (status == 'loaded') {
		const data2 = res2[0];
	}

    // This also does not, as it thinks dataFromDestructuring is number|null inside the type narrowing
    const [dataFromDestructuring, statusFromDestructuring] = res2
	if (statusFromDestructuring == 'loaded') {
		console.log(dataFromDestructuring)
	}
}

So narrowing directly on the tuple type works fine, but when destructuring or assigning to an intermediate variable, it does not narrow correctly.

@squidfunk
Copy link
Author

@GeeWee: I think that those are effectively two separate bugs, as the issue describes type narrowing without destructuring on tuples with primitive (working) and non-primitive (not working) values.

@GeeWee
Copy link

GeeWee commented Sep 3, 2019

I think you're probably right actually.

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Sep 3, 2019
@DanielRosenwasser
Copy link
Member

Looks like this is probably a bug in isMatchingReference.

@DanielRosenwasser DanielRosenwasser added the Domain: Control Flow The issue relates to control flow analysis label Sep 3, 2019
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.7.0 milestone Sep 3, 2019
@Kingwl
Copy link
Contributor

Kingwl commented Sep 13, 2019

I'd like to try this if no one started work

@sandersn
Copy link
Member

Sounds good, I wasn't planning to start on it until after the beta starts.

@sandersn
Copy link
Member

This is actually not specific to tuples. Control-flow narrowing doesn't work with any nested properties:

interface A { data: "a" }
interface B { data: "b" }
interface C { data: "c" }

type Y = { a:A, b:C } | { a:B, b:B }

function foo(y: Y) {
  if (y.a.data === "b") {
    return y.b.data === "c"
  }
}

It probably could be done but would require us to either (1) re-think handling of unions of object types or (2) how to relate entity-named discriminants that share a prefix.

@sandersn sandersn modified the milestones: TypeScript 3.8.1, Backlog Jan 10, 2020
@sandersn sandersn removed their assignment Jan 10, 2020
@sandersn sandersn added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Jan 10, 2020
yaacovCR added a commit to yaacovCR/graphql-js that referenced this issue Jun 10, 2022
See open TS issue microsoft/TypeScript#31613

Marked with FIXME for when TS feature/bug fix lands upstream

[This code will likely be removed in v17 prior to the feature landing, but may be retained within v16 for longer.]

Similar code "works" at https:/graphql/graphql-js/blob/1f8ba95c662118452bd969c6b26ba4e9050c55da/src/error/GraphQLError.ts#L47 because all the option properties are optional.
@jtbandes
Copy link
Contributor

Possibly related: #48378

@squidfunk
Copy link
Author

3 years old, still not fixed. Is this issue dead? It was first scheduled for 3.7 and then repeatedly rescheduled until it ended in the backlog. Fixing this would help in several use cases and greatly improve expressivity. I have several parts in my code base where I need to fall back to very generic types that are not helpful at all what could be a union of tuples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Control Flow The issue relates to control flow analysis Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented.
Projects
None yet
Development

No branches or pull requests

8 participants