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

Invoking sendMessage('key') without passing undefined #23

Closed
noamloewenstern opened this issue Mar 19, 2023 · 2 comments
Closed

Invoking sendMessage('key') without passing undefined #23

noamloewenstern opened this issue Mar 19, 2023 · 2 comments
Labels
wontfix This will not be worked on

Comments

@noamloewenstern
Copy link
Contributor

In the docs there's an example of invoking sendMessage with a key which doesn't need to pass data, therefore needs to pass 'undefined'.
Is there a way in TS to auto-recognize that there's no need to pass data and there won't demand passing 'undefined'?
I've searched the internet, and seems there's a difference between making an argument optional and explicitly allowing an undefined type.
For example:
If the ProtocolMap is:

export interface ProtocolMap {
  foo: { id: number; name: string };
  bar: ProtocolWithReturn<string, number>;
  noDataKey: ProtocolWithReturn<undefined, number>;
}

Even if I want to call 'noDataKey' like this:

sendMessage('noDataKey')

I can't, TS error says: expected 2-3 arguments, but got 1. An argument for 'data' was not provided
And so I have to pass undefined:

sendMessage('noDataKey', undefined)

I've realized that this issue of assuming undefined type as optional has been a widespread issue, but haven't managed to find out if there's a way around it, or if there's another way to address this issue.
The links I found regarding this issue:
Here
Here
Here
And even in the Design Meeting Notes, adding a screenshot of the exact address of this issue:
image


I found that explicitly declaring an argument as void will allow this!
Like so:

function func1(name: string, age: number | undefined) {}
func1('a') // will show error "Expected 2 arguments, but got 1"

function func2(name: string, age: number | void) {}
func2('a') // will not show any error!

But trying to apply this to the GetDataType did not allow, even if TS knows it's a void.

interface ProtocolMap {
  noDataKey: ProtocolWithReturn<void, number>;
}
// ...
sendMessage('noDataKey') // Still error "Expected 2-3 arguments", even though ts recognizes it as void, as the following image shows:

image

Maybe worth looking into it?

@aklinker1
Copy link
Owner

I could easily add an override that would allow only 1 argument to be passed:

function sendMessage<T extends KeyOfWithNoData<ProtocolMap>>(
  type: T,
  tabId?: number,
): Promise<GetReturnType<ProtocolMap, T>>;

function sendMessage<T extends keyof ProtocolMap>(
  type: T,
  data: GetDataType<ProtocolMap, T>,
  tabId?: number,
): Promise<GetReturnType<ProtocolMap, T>>;

The KeyOfWithNoData type is complex, I've hidden the implementation in this example. Just know compared to keyof, it would only return keys who's data is specified as undefined.

In fact, I did this for some projects at work when I wrote a messaging wrapper for them before I wrote this library.

So the problem isn't making the function accept a single parameter. Rather, the reason I didn't add this is because the final parameter, tabId, is ambiguous.

It's impossible to determine what the intent is for a call like this:

sendMessage("someMessage", 123);

Am I sending a message that doesn't have any payload to tab id=123 or am I sending a message with data=123? Internally, the code has no way of knowing since it can't inspect types at runtime.

So I've decided to require a data parameter, even if it's undefined, to be explicit.

@noamloewenstern
Copy link
Contributor Author

Interesting. Alright. Thanks!

@aklinker1 aklinker1 added the wontfix This will not be worked on label Mar 20, 2023
@aklinker1 aklinker1 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants