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

Overly-narrowed generic object loses typing #57804

Closed
nmussy opened this issue Mar 16, 2024 · 2 comments
Closed

Overly-narrowed generic object loses typing #57804

nmussy opened this issue Mar 16, 2024 · 2 comments

Comments

@nmussy
Copy link

nmussy commented Mar 16, 2024

🔎 Search Terms

"generic narrowing", "union narrowing", "generic guard", "ts2345"

🕗 Version & Regression Information

  • This is the behavior in every version I tried (from v3.9 to v5.5.0-dev.20240316), and I reviewed the FAQ for entries about type narrowing
  • I was unable to test this on prior versions because Property 'values' does not exist on type 'ObjectConstructor'.(2339)

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.5.0-dev.20240316#code/MYGwhgzhAEDCD2BbR8B20DeBfAUKSMAgtAKYAeALiagCYwLJqY4CQADgK4BGIAlsNABOJMDTQgAntDaD4bYgF5oAImUBuHLnxRoAIVKVqdOEhToMrTj35CRY1JOmy2+pao24cFCWxInGqACqFLwgAGKoEAA8ACoGVLT0pmgAfNBKFiy8EITwgroAXNAAFABuYCBFDGYAlOlp5SDQ2dAxGiwA5iQUAArORWUVRTF1CmkYTnJFEBSCvKgd0FgeGnhoM9AcISAwGayERZlZOXmFJaUkghC8aFXJqDVFF1c36C2KaQCEn2WX10yQaCEGoAOhkckIABpWJ1un0puc-q8isD6iUJuC2E8kWgwc5iFgatCWFhoBAwCEIAAzXgkJIBYKhCLRQgpYlnI7ZXL5AbPf6oO4BR7QPmvZowVxfH6igES0GY3TE2G9fqIl63PSjNLFDGqmWoPFyfSE4mk8mUml0-xmRnhSJRXRszSrTLAdYUaBdFVydLnIbWtBa5gsFhUvIlN2RD1bULQeBU6AAeS4ACsSMAKCDGhw6cUYzsanUjlkE3ntiCuadBiBC7YKBxBOh8yCvfC2NWau0STDhPXG9BUBwQCB2stNDgcK73Z64c5fdXBWZIZtthBF2hbcyotVUgBtAC6QaOYcEEen+bjCfzEDqvFLzcr+Q7dV7Dab5dbzg7Xdf-cHw9HDwJynKMZ29Nh50addUGXa9oM3e1iAAHz0FIDyPGETzPUCL3jFdQhvZp73LR9dGfOs33wkAW1nORvx7bpKP-EdWDHXAgA

💻 Code

class Common {}
class A extends Common {
	public readonly propA = "";
}
class B extends Common {
	public readonly propB = "";
}

type CommonUtilFns<T extends Common> = {
	isAorB: (val: Common) => val is T;
	getProp: (val: T) => { prop: string };
};

const utils = {
	A: {
		isAorB: (version: Common): version is A => !!(version as A).propA,
		getProp: (version: A) => ({ prop: version.propA }),
	} satisfies CommonUtilFns<A>,
	B: {
		isAorB: (version: Common): version is B => !!(version as B).propB,
		getProp: (version: B) => ({ prop: version.propB }),
	} satisfies CommonUtilFns<B>,
};

const getProp = (val: Common) => {
	for (const util of Object.values(utils)) {
		if (util.isAorB(val)) return util.getProp(val);
	}
	return null;
};

🙁 Actual behavior

Invoking util.getProp after guarding it with util.isAorB results in a type error:

Argument of type 'A | B' is not assignable to parameter of type 'A & B'.
  Type 'A' is not assignable to type 'A & B'.
    Property 'propB' is missing in type 'A' but required in type 'B'.(2345)

The return value of Object.values seems to be correctly typed, as this is the evaluated typing of util:

const util: {
    isAorB: (version: Common) => version is A;
    getProp: (version: A) => {
        prop: string;
    };
} | {
    isAorB: (version: Common) => version is B;
    getProp: (version: B) => {
        prop: string;
    };
}

However, once val is guarded, its type becomes A | B

🙂 Expected behavior

I would expect the type guard to correctly narrow the type as A | B, given I'm using the same instance of CommonUtilFns with the same generic.

This however works correctly with these examples:

const getProp = (val: Common, utils: CommonUtilFns<Common>[]) => {
	for (const util of utils) if (util.isAorB(val)) return util.getProp(val);
	return null;
};
const getProp = (val: Common, utils: CommonUtilFns<A | B>[]) => {
	for (const util of utils) if (util.isAorB(val)) return util.getProp(val);
	return null;
};

Additional information about the issue

This might be related to #17713, but I haven't found another issue specifically addressing this pattern, apologies if I missed it

@jcalz
Copy link
Contributor

jcalz commented Mar 16, 2024

Your bottom examples "work" because CommonUtilFns<Common> and CommonUtilFns<A | B> is not the same as CommonUtilFns<A> | CommonUtilFns<B>. You can't just distribute across unions. CommonUtilFns<T> is invariant in T:

declare let f: CommonUtilFns<A> | CommonUtilFns<B>
declare let g: CommonUtilFns<A | B>
f = g; // error!
g = f; // error!

Your top example doesn't work because it's essentially #30581. TS does not keep track of the identity of util here, just its type. If you have util1 and util2 both of type CommonUtilFns<A> | CommonUtilFns<B>, then if (util1.isAorB(val)) return util2.getProp(val) wouldn't be safe, right? That's the problem TS has. And that's been reported in #30581 and the recommended approach is detailed at #47109. That looks like the following:

interface UtilMap {
	A: A;
	B: B;
}

const utils: { [K in keyof UtilMap]: CommonUtilFns<UtilMap[K]> } = {
	A: {
		isAorB: (version: Common): version is A => !!(version as A).propA,
		getProp: (version: A) => ({ prop: version.propA }),
	},
	B: {
		isAorB: (version: Common): version is B => !!(version as B).propB,
		getProp: (version: B) => ({ prop: version.propB }),
	}
};

function apply<K extends keyof UtilMap>(util: (typeof utils)[K], val: Common) {
	return util.isAorB(val) ? util.getProp(val) : undefined
}

{
	const getProp = (val: Common) => {
		for (const util of Object.values(utils)) {
			const ret = apply(util, val);
			if (ret) return ret;
		}
		return null;
	};
}

Which, yes, is a lot of weird generic stuff. See #47109. Anyway I'd say this is essentially a duplicate of #30581.

Playground link

@nmussy
Copy link
Author

nmussy commented Mar 17, 2024

Thanks for the detailed explanation @jcalz! Your apply function was also more or less what I got recommended by Copilot, which is to do both the guarding and the prop extraction in a single function, with the same generic context. It's an understandable limitation, but a little frustrating because TS gets so close to getting it right.

I'm going to close this issue. Maintainers, feel free to mark it as a duplicate. And thanks again for your time Joe 👍

@nmussy nmussy closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants