aklinker1 / webext-core

Collection of essential libraries and tools for building web extensions
https://webext-core.aklinker1.io
MIT License
96 stars 11 forks source link

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

Closed NoamLoewenstern closed 1 year ago

NoamLoewenstern commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Interesting. Alright. Thanks!