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

Unable to create a function wrapper #54569

Closed
stronny opened this issue Jun 8, 2023 · 10 comments
Closed

Unable to create a function wrapper #54569

stronny opened this issue Jun 8, 2023 · 10 comments

Comments

@stronny
Copy link

stronny commented Jun 8, 2023

Bug Report

πŸ”Ž Search Terms

2556, tuple, union

πŸ•— Version & Regression Information

n/a

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

	class Test {
		one(s: string): void {s}
		two(n: number): boolean {return n > 100}
		ten(k: CryptoKey, w: Window): Document {k; return w.document}
	}
	const test = new Test()
	export function run<Name extends keyof Test, Args extends Parameters<Test[Name]>>(name: Name, ...args: Args): void {
		const fn = test[name]
		fn(...args)
	}

	run('one', '')
	run('two', 123)
	run('ten', null, window)

πŸ™ Actual behavior

TypeScript shows an error about the spread argument.

I've seen a bunch of similar issues, however I left unsatisfied with them. My specific concerns are these:

  1. Okay, so how do I create a wrapper function that preserves the static analysis?
  2. apply doesn't work as well, with a different error.
  3. Despite the error, TS seems to actually understand me and check the arguments of the wrapped functions, in particular:
  • I can't call run('five')
  • I can't call run('one')
  • I can't call run('one', 123)
  • I can't call run('ten', null, window) as demonstrated in the last line
  1. TS doesn't provide any facilities to ignore this particular error that I know is irrelevant to me.

I'm left with no options, and I think this deserves an issue.

πŸ™‚ Expected behavior

TypeScript doesn't show an error.

@fatcerberus
Copy link

fatcerberus commented Jun 8, 2023

Okay so the problem here is that Name extends keyof Test can be a union of keys - in which case name and args can be mismatched1, so TS is warning you that the call is unsafe for that reason.

You really need #27808 to deal with this properly. Short of that, you’ll just have to ts-ignore the error.

Footnotes

  1. Put more generally, it is not safe to call a union of function types with a union of their parameter types. ↩

@stronny
Copy link
Author

stronny commented Jun 8, 2023

TS is warning you that the call is unsafe for that reason.

Nevertheless it does typecheck the calls correctly, so everything works exactly as I expect it to, except the error message.

@fatcerberus
Copy link

fatcerberus commented Jun 8, 2023

Yes, but this call also typechecks:

run("one" as "one" | "two", 123);

…which therefore makes the inner call to fn unsafe. Hence the error message.

@jcalz
Copy link
Contributor

jcalz commented Jun 8, 2023

  1. Okay, so how do I create a wrapper function that preserves the static analysis?

I'd say #47109 is the current way to express this sort of thing:

type TestArgs = { [K in keyof Test]: Parameters<Test[K]> }
type TestRet = { [K in keyof Test]: ReturnType<Test[K]> }
const test: { [K in keyof Test]: (...args: TestArgs[K]) => TestRet[K] } = new Test();
export function run<K extends keyof Test>(name: K, ...args: TestArgs[K]): void {
	const fn = test[name]; 
	const ret = fn(...args);
	//    ^? const ret: TestRet[K]
}

Playground link to code

  1. TS doesn't provide any facilities to ignore this particular error that I know is irrelevant to me.

Could you elaborate? On the face of it it looks false: TS provides type assertions, the any type, and the //@ts-ignore directive, each of which could be used to prevent your code from complaining. I'm not recommending you use any of those, but I'm wondering why they don't count as "facilities to ignore" your error.

@stronny
Copy link
Author

stronny commented Jun 8, 2023

run("one" as "one" | "two", 123);

…which therefore makes the inner call to fn unsafe. Hence the error message.

I'm afraid I don't understand what is unsafe here. It correctly points out that you can't run one with a number.

I'd say #47109 is the current way to express this sort of thing:

I am once again finding myself at the bottom of the confidence curve. Hats off, this seems to check out.

  1. TS doesn't provide any facilities to ignore this particular error that I know is irrelevant to me.
    Could you elaborate? On the face of it it looks false: TS provides type assertions, the any type, and the //@ts-ignore directive, each of which could be used to prevent your code from complaining. I'm not recommending you use any of those, but I'm wondering why they don't count as "facilities to ignore" your error.

Which one of these can tell TS to ignore this particular error and nothing else, again? @ts-ignore ignores the whole line, so other errors can slip in. I run strict, so any any is an error in itself. I tried to assert the type of ...args, but came back empty-handed too.

@fatcerberus
Copy link

fatcerberus commented Jun 8, 2023

I'm afraid I don't understand what is unsafe here. It correctly points out that you can't run one with a number.

It does not: See Playground

@jcalz Worth noting that your solution suffers from the exact same unsoundness as shown here, despite removing the error in the implementation. This pattern really does beg for oneof constraints.

@stronny
Copy link
Author

stronny commented Jun 8, 2023

It does not

I see the problem now. Strange how run('one', 123) gives an error, but run('one' as keyof Test, 123) doesn't, despite the same type being declared in the function argument. My TS knowledge is not enough to reason about what's going on at this point.

As it's my own code, this solution still seems perfectly safe for my particular use case.

@fatcerberus
Copy link

In the first case, name is typed as "one"; in the second case it's typed as keyof Test. That's the difference.

By giving a union of keys for Name, you get a union of function types for Test[Name]. Then, because Parameters is distributive, it resolves to a union of those functions' parameter types. TS doesn't understand that these unions are all supposed to correlated (there's a separate issue about that, fwiw). By passing a string literal directly, it infers a string literal type for Name instead of just keyof Test, TS then knows exactly which function Test[Name] refers to, and you get your nice type checking on the call.

@stronny
Copy link
Author

stronny commented Jun 8, 2023

In the first case, name is typed as "one"; in the second case it's typed as keyof Test. That's the difference.

That actually makes sense. Thank you.

I suppose I should go ahead and close the issue now.

@stronny stronny closed this as completed Jun 8, 2023
@jcalz
Copy link
Contributor

jcalz commented Jun 8, 2023

@jcalz Worth noting that your solution suffers from the exact same unsoundness as shown here, despite removing the error in the implementation. This pattern really does beg for oneof constraints.

Yeah, see #48730

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

3 participants