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

Allow dictionaries and callbacks to be distinguishable #1191

Closed
jakearchibald opened this issue Sep 16, 2022 · 8 comments · Fixed by #1353
Closed

Allow dictionaries and callbacks to be distinguishable #1191

jakearchibald opened this issue Sep 16, 2022 · 8 comments · Fixed by #1353

Comments

@jakearchibald
Copy link
Contributor

https://webidl.spec.whatwg.org/#dfn-distinguishable

It isn't clear to me why callbacks and dictionaries can't be distinguished.

I'm designing an API like:

whatever(callback);

But there's a strong possibility that options will be added in future, and I want to avoid this pattern:

whatever(() => {
  // Quite
  // a
  // bit
  // of
  // code
}, { behaveDifferently: true });

And instead go for:

whatever({
  behaveDifferently: true,
  callback() {
    // Quite
    // a
    // bit
    // of
    // code
  }
});

But it seems like I can't overload from whatever(callback) to whatever({ callback }).

I would have assumed that an argument could be treated as a callback if it's callable.

@bathos
Copy link
Contributor

bathos commented Sep 17, 2022

It seems like both the union conversion & overload resolution algorithms would already behave sensibly if a dot were added to that grid as both have if IsCallable(V) and there’s a... branches before dictionary.

I thought maybe [LegacyTreatNonObjectAsNull] might have been a factor, but it only does its weird thing when nullable and that would preclude it from being in a union with a dictionary in the cases where it needs to be regardless. I’m less certain if that’s also true for overload distinguishability already.

@jeremyroman
Copy link
Contributor

I suspect this change could be made in a web-compatible way, but FWIW you could technically use a callback function as a dictionary, since functions are objects.

let silly = Object.assign(e => {}, {once: true});
document.body.addEventListener('click', silly, silly);

Probably just making these distinguishable and keeping the current logic whereby the IsCallable check happens before the dictionary conversion.

As a consistency point, EventTarget/addEventListener looks more like your second example -- but I don't think that pattern is ubiquitous.

@bathos
Copy link
Contributor

bathos commented Sep 17, 2022

and keeping the current logic whereby the IsCallable check happens before the dictionary conversion

That’s what I’d expect — the same “fall through” pattern as sequence vs dictionary (or boolean vs number, etc).

While I don’t see any trouble for signatures defined this way from the start, the allowance might create the false impression that changing existing signatures in this manner is safe. Maybe a note that explains why extending established dictionary-only signatures through overloading/unions which add callback functions (or sequences) carries a back-compat risk would help?

I don’t think it changes what was intended to be conveyed, so this is just FYI: in the Web IDL sense of the term, addEventListener doesn’t take a “callback function” but rather a “callback interface”. Those really couldn’t be distinguished from dictionaries: {} is a valid EventListener for example. The “operation member” (a handleEvent method in this case) can be sprouted/removed at will — it’s not “captured” like dictionary members.

@jakearchibald
Copy link
Contributor Author

@jeremyroman

I suspect this change could be made in a web-compatible way, but FWIW you could technically use a callback function as a dictionary, since functions are objects.

Ohh interesting. I could probably hack this through in the prose.

As a consistency point, EventTarget/addEventListener looks more like your second example -- but I don't think that pattern is ubiquitous.

Yeah, there's also setTimeout(callback, delay), but I think these should be considered anti-patterns for newer APIs.

@jakearchibald
Copy link
Contributor Author

@bathos

While I don’t see any trouble for signatures defined this way from the start, the allowance might create the false impression that changing existing signatures in this manner is safe.

Is it any more risky than taking a string argument and introducing an object type? That means code already using that object in the wild will go from being toString'd to something else.

Although, I guess in the wild, folks might be using a constructor function as a dictionary (via 'static' properties). That would pretty significantly change behaviour.

just FYI: in the Web IDL sense of the term, addEventListener doesn’t take a “callback function” but rather a “callback interface”

Yeah, it seems like the "callback interface" stuff is only for legacy use.

@annevk
Copy link
Member

annevk commented Sep 20, 2022

It would be good to fix #100 to make the legacy aspect of those clearer. (I'm surprised this is the first issue on making callbacks and dictionaries distinguishable. I looked around a bit and couldn't find anything.)

@vmpstr
Copy link
Member

vmpstr commented Aug 16, 2023

I've put up a PR to try and address this. I would appreciate comments: #1353

@jakearchibald
Copy link
Contributor Author

Yay, great to see this land!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants