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

Generic type unnarrowed by switch...case #43873

Open
brianjenkins94 opened this issue Apr 28, 2021 · 10 comments
Open

Generic type unnarrowed by switch...case #43873

brianjenkins94 opened this issue Apr 28, 2021 · 10 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@brianjenkins94
Copy link

brianjenkins94 commented Apr 28, 2021

Bug Report

🔎 Search Terms

  • narrowing of generic types
  • exhaustive switch case

🕗 Version & Regression Information

Theoretically this should be fixed by:

Similar issue:

I'm trying against the latest beta: 4.3.0-pr-43183-11 / 15fae38.

⏯ Playground Link

Playground link with relevant code

💻 Code

interface TopLevelElementMap {
	"a": HTMLElement;
	"b": HTMLElement;
}

interface ElementAttributes {}

interface ElementAttributesMap {
	"a": ElementAttributes;
	"b": ElementAttributes;
}

class Element<TagName extends keyof TopLevelElementMap> {
	protected attributes: ElementAttributesMap[TagName] = {};

	public constructor(type: TagName) {}
}

const ElementTagNameMap = {
	"a": function(): Element<"a"> {
		return new Element("a");
	},
	"b": function(): Element<"b"> {
		return new Element("b");
	}
};

function createPrimitive<TagName extends keyof typeof ElementTagNameMap>(tagName: TagName) {
	switch (tagName) {
		case "a":
			return function(): Element<TagName> {
				//
				// [?] Shouldn't `tagName` get narrowed to just "a"?
				//
				return ElementTagNameMap[tagName]();
			};

		case "b":

		default:
			throw new Error("Unrecognized element `" + tagName + "`.");
	}
}

const a = createPrimitive("a");
const b = createPrimitive("b");

As a workaround I can assert as Element<TagName> on the inner-most return statement to silence the error.

🙁 Actual behavior

Type 'Element<"a"> | Element<"b">' is not assignable to type 'Element<TagName>'.
  Type 'Element<"a">' is not assignable to type 'Element<TagName>'.
    Type '"a"' is not assignable to type 'TagName'.
      '"a"' is assignable to the constraint of type 'TagName', but 'TagName' could be instantiated with a different subtype of constraint '"a" | "b"'. (2322)

🙂 Expected behavior

TagName should be narrowed to one of the possible values dictated by the case clause.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 29, 2021

Currently we don't contextually type the "argument" of an indexing element access expression - but you would only want to do that when the element access itself is contextually typed by a non-generic type the same way as described in #43183

@brianjenkins94
Copy link
Author

brianjenkins94 commented Apr 29, 2021

I must have been staring at this for too long. This works fine:

Playground

function createPrimitive(tagName: keyof typeof ElementTagNameMap) {
	switch (tagName) {
		case "a":
			return function(): Element<"a"> {
				return ElementTagNameMap[tagName]();
			};

		case "b":
		case "c":
			return function(): Element<"b" | "c"> {
				return ElementTagNameMap[tagName]();
			};

Although it is a bit redundant.

@brianjenkins94
Copy link
Author

brianjenkins94 commented Apr 29, 2021

Another case where I'm expecting something to be narrowed and it isn't:

Playground

Surely there's a better way to write this than by explicitly reiterating the case clause in the type:

function createPrimitive(tagName: keyof typeof ElementTagNameMap) {
	switch (tagName) {
		case "audio":
			return function(attributes: ElementAttributesMap["audio"] = {}): Element<"audio"> {
				//                                       ^^^^^^^                 ^^^^^^^
				attributes.autoplay = true;

				return ElementTagNameMap[tagName](attributes);
			};

@brianjenkins94
Copy link
Author

@DanielRosenwasser Are both of my problems related to the same "indexing element access expression" issue you described?

Do you have any suggestions on how to better approach this?

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Apr 29, 2021
@RyanCavanaugh
Copy link
Member

There are a bunch of problems here, only one of which is in-principle addressable as a novel thing:

  • tagName is mutable, so narrowings on it don't apply in closures. Reassigning it to a const is necessary here at a minimum
  • e[x] when x is a type parameter constrained to K1 | K2 and K1 and K2 are both valid keys of e should be legally narrowed via a contextual type <-- my interpretation of what this issue is
  • The utterance Element<TagName> in the return type of the function expression isn't equivalent to Element<"a">; narrowings of values of a generic type do not imply narrowings of the representing generic, nor should they (it only implies an upper bound). Properly handling this is effectively Suggestion for Dependent-Type-Like Functions: Conservative Narrowing of Generic Indexed Access Result Type #33014

@brianjenkins94
Copy link
Author

brianjenkins94 commented Apr 30, 2021

Okay. People keep saying I have a bunch of problems but I'm blind to them so I'm going to see what I can do about figuring them out on my own in hopes of helping out the next guy.

This is my current level of understanding when it comes to generics:

https://ts.chibicode.com/generics/

Working off of this playground to start, I am using generics in 3 places:

  1. As part of my abstract Element class:

    export abstract class Element<TagName extends keyof TopLevelElementMap> {
    	protected template: HTMLElement;
    	protected attributes: ElementAttributesMap[TagName] = {};
    	protected readonly type: TagName;
    	protected readonly children: string[] = [];
    	private events = {};
    
    	public constructor(type: TagName) {
    		this.type = type;
    
    		this.template = document.createElement(this.type);
    	}
    }
    • I declare TagName as a type that extends keyof TopLevelElementMap.
    • TopLevelElementMap is an interface that relates tag names (string keys) to their element types.
      • e.g: "a": HTMLAnchorElement; (where HTMLAnchorElement comes from lib.dom.d.ts)
    • I don't think there's anything that can be wrong here.
  2. As part of my primeConstructor function:

    function primeConstructor<TagName extends keyof TopLevelElementMap, ArgsType extends unknown[]>(Node: new (type: TagName, ...args: ArgsType) => Element<TagName>, type: TagName): (...args: ArgsType) => Element<TagName> {
    	return function(...args: ArgsType): Element<TagName> {
    		return new Node(type, ...args);
    	};
    }
    • primeConstructor was only ever intended to save me some repetition in not having to specify the tag name when creating new nodes.
    • It has the same definition of TagName as above.
      • Is declaring this in two separate places breaking the relationship I'm trying to build?
    • It additionally declares ArgsType as something that is supposed to gobble up the rest of the parameters fed into the primeConstructor function and preserve the type information which ultimately gets fed into the Node constructor.
      • Should I be using ConstructorParameters<> to get the arguments of the Node as a tuple? This very much feels like its use case.
    • This returns a factory function for creating Nodes.
      • I think I'm losing type information here, but I don't think that relates to either of the problems I'm encountering.
  3. As part of my createPrimitive function:

    function createPrimitive<TagName extends keyof typeof ElementTagNameMap>(tagName: TagName) {
    	// Per @RyanCavanaugh's first bullet:
    	const tagNameImmutable = tagName;
    	
    	switch (tagNameImmutable) {
    • Declare TagName as a type again, but this time as keyof typeof ElementTagNameMap which is an object of the same keys in TopLevelElementMap but instead of mapping tag names to element types, it maps tag names to the factory functions returned as the result of calling primeConstructor.
      • Maybe this is a relationship that I need to explicitly codify somehow?

Here's where things seem to be going awry:

function createPrimitive<TagName extends keyof typeof ElementTagNameMap>(tagName: TagName) {
	// Per @RyanCavanaugh's first bullet:
	const tagNameImmutable = tagName;
	
	switch (tagNameImmutable) {
		case "a":
			return function(attributes: ElementAttributesMap[TagName] = {}): Element<TagName> {
				attributes.href = "whatever";
				// [?] Shouldn't `attributes` get narrowed to just `AnchorElementAttributes`?

				return ElementTagNameMap[tagName](attributes);
				// [?] Shouldn't `() => Element<"a">` satisfy the constraint of `Element<TagName>`?
			};
  • createPrimitive returns a factory function that invokes the factory function that we created by invoking primeConstructor.

Problem 1: Element<TagName> doesn't behave the way I expect it to.


Problem 2: ElementAttributesMap[TagName] doesn't behave the way I expect it to.

  • Expectation: TagName indexes ElementAttributesMap and types attributes as ElementAttributesMap["a"].

  • I think I need a better understanding of dependent types and discriminated unions.


Problem 3: The compiler isn't currently designed to handle this: ElementTagNameMap[tagName](attributes)

  • Expectation: ElementTagNameMap[tagName] becomes the result of primeConstructor(AnchorElement, "a") and invokes it.

  • I think this is the bit that DanielRosenwasser was pointing out.

    Currently we don't contextually type the "argument" of an indexing element access expression - but you would only want to do that when the element access itself is contextually typed by a non-generic type the same way as described in Improve narrowing of generic types in control flow analysis #43183

@brianjenkins94
Copy link
Author

I have resolved that this is the best way to handle my use case for now:

case "a":
	return function(attributes: ElementAttributesMap["a"] = {}): Element<"a"> {
		attributes.href = "whatever";

		return ElementTagNameMap[tagName](attributes) as Element<"a">;
	};

and that this may improve in the future with #33014.

@mkantor
Copy link
Contributor

mkantor commented Jul 1, 2021

Is there a more targeted issue for just this problem?

  • e[x] when x is a type parameter constrained to K1 | K2 and K1 and K2 are both valid keys of e should be legally narrowed via a contextual type

In case it helps, here's a simpler situation that demonstrates it without the other problems mentioned by @RyanCavanaugh:

const stuff = {
    string: '',
    number: 0,
}
function f<T extends 'string' | 'number'>(x: T): void {
    if (x === 'string') {
        stuff[x].length // error :(
    }
}

@brianjenkins94
Copy link
Author

Does Control Flow Analysis of Aliased Conditions help with this at all?

@ouuan
Copy link

ouuan commented Aug 26, 2021

I found a case that could be relevant: Playground link.

I've confirmed that in this piece of code, const a: "a" = x, takesA(x) and const never: never = x have errors in TypeScript v4.2.3. But in v4.3.5, only obj[x] have an error. This could be the result of #13995 being partially fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants