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

Please prefix getter/setter/callback interface with legacy so it's clear they are #100

Open
annevk opened this issue Mar 21, 2016 · 18 comments
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛ duration:medium Shouldn't be too long to fix editorial Changes that do not affect how the standard is understood

Comments

@annevk
Copy link
Member

annevk commented Mar 21, 2016

This cannot be fixed soon enough. Apparently we now have RTCIdentityProvider as callback interface? That should never have been added to the platform. We should really mark all the getter/setter/callback interface/etc. stuff as legacy so it's not used.

@bzbarsky
Copy link
Collaborator

Why is callback interface supposed to be legacy? Except for the single-function case (which should use a callback instead), its a perfectly reasonable API design. The other option is a list of callbacks, which has its own benefits and drawbacks.

Note that the consumer can shim a "list of callbacks" interface on top of a callback interface by creating a dummy object, just like one can shim a "callback interface" interface on top of a list of callbacks by binding all the methods to the "this" object and passing them in. So in practice the API design choice should be mostly informed by how many callbacks there are and whether it makes sense to have them be methods of a single object conceptually.

For the case of one callback, clearly just a callback function is better. For two callbacks, unclear. For 10 callbacks, I would argue that a callback interface is a much better representation than a flat list; at least as an API consumer I would prefer to not have to count my function arguments to see which one is which.

@bzbarsky
Copy link
Collaborator

Ah, I see the suggestion for dictionary of callbacks in the other bug. That's probably good enough, yeah... Can't do getters or setters that way, but survivable.

@foolip
Copy link
Member

foolip commented Mar 21, 2016

It seems like a big plus with dictionaries that the members are evaluated immediately, but are there reasons to ever prefer the "lazy" behavior of callback interfaces?

It looks within reach to limit callback interfaces to the three existing cases, which are all single-function cases.

@jan-ivar
Copy link
Contributor

One reason might be to not put on the user to remember to .bind(this) on every callback.

@annevk
Copy link
Member Author

annevk commented Mar 21, 2016

@jan-ivar most callbacks don't require that at all.

@jan-ivar
Copy link
Contributor

Right, I didn't mean all callbacks, I meant every callback in a particular interface that now has to register its methods, like w3c/webrtc-pc#559 (comment).

Not saying it's a big reason, but since we were asking.

@annevk
Copy link
Member Author

annevk commented Apr 14, 2016

Note: WebRTC got fixed.

@tobie tobie added ⌛⌛ duration:medium Shouldn't be too long to fix ☕☕ difficulty:medium Hard to fix editorial Changes that do not affect how the standard is understood labels Jun 19, 2016
@annevk annevk changed the title Please rename things that are legacy so it's clear they are Please prefix getter/setter/callback interface with legacy so it's clear they are Aug 22, 2017
@tobie
Copy link
Collaborator

tobie commented Aug 22, 2017

Worth noting somewhere that the only Legacy callback interface object is NodeFilter.

@annevk
Copy link
Member Author

annevk commented Aug 22, 2017

That's the only one with constants. EventListener is another and I think there might be more.

@tobie
Copy link
Collaborator

tobie commented Aug 22, 2017

Yeah. There are more callback interfaces but just one legacy callback interface object.

@annevk
Copy link
Member Author

annevk commented Aug 22, 2017

No, they're all legacy.

@tobie
Copy link
Collaborator

tobie commented Aug 22, 2017

Yeah, they're all legacy, but only NodeFilter is a Legacy callback interface object.

@RByers
Copy link

RByers commented Feb 14, 2018

Is it really true that getter/setter is "legacy" and discouraged in new APIs? Can anyone provide a reference for this documenting the extent of the consensus here? In the absence of agreed upon guidance (eg. captured concretely in the webidl spec), I'm personally not willing to block requests to ship such new APIs in blink (such as Typed OM).

/cc @ajklein @foolip @chrishtr

@bzbarsky
Copy link
Collaborator

I think there is general agreement that named getters/setters are a bad idea.

For indexed getters/setters, things are complicated, not least because there are legitimate use cases that have no other solutions right now. Indexed setters do require some careful spec prose around out-of-bounds sets.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Feb 14, 2018

One other thing. I think there is a large camp that considers some of the API goals of Typed OM as currently specified to be broken by design. Not the actual functionality, but the use of typing and typechecking. I personally am not in this camp, so I do not want to speak for it, but just wanted to mention its existence. This is part of the issue you are encountering with pushback on the Typed OM use of indexed getters/setters.

@RByers
Copy link

RByers commented Feb 14, 2018

For indexed getters/setters, things are complicated, not least because there are legitimate use cases that have no other solutions right now. Indexed setters do require some careful spec prose around out-of-bounds sets.

Thanks. This case is indexed getters / setters only.

I think there is a large camp that considers some of the API goals of Typed OM as currently specified to be broken by design. Not the actual functionality, but the use of typing and typechecking. I personally am not in this camp, so I do not want to speak for it, but just wanted to mention its existence. This is part of the issue you are encountering with pushback on the Typed OM use of indexed getters/setters.

Thanks for the heads up. Can you point to anywhere that this feedback has been given publicly? I didn't come across it when scanning the open issues or initial TAG feedback, but I only did a quick scan so I may have missed it.

@bzbarsky
Copy link
Collaborator

I don't have links offhand, sorry. It's the general "should there be eager or lazy typechecking" problem that's been discussed a few times on es-discuss and public-script-coord. And probably in person in TC39 and private discussions....

Some of this is referenced in the long discussion at #345 (again, no links there). There are references to it in the TAG feedback issue.

@tabatkins
Copy link
Contributor

some of the API goals of Typed OM as currently specified to be broken by design. Not the actual functionality, but the use of typing and typechecking.

And note, of course, that this isn't a special goal of Typed OM, it's part and parcel of literally all of WebIDL.

I think there is general agreement that named getters/setters are a bad idea.

For indexed getters/setters, things are complicated, not least because there are legitimate use cases that have no other solutions right now. Indexed setters do require some careful spec prose around out-of-bounds sets.

I strongly agree that named getters/setters should be considered legacy and/or given a name that clearly demonstrates they're special-use and should have thoro review of the necessity. Maps satisfy all their use-cases much better.

For indexed getters/setters, yeah, I'm finding there's a lot of missing guidance on how to do these right. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛ duration:medium Shouldn't be too long to fix editorial Changes that do not affect how the standard is understood
Development

No branches or pull requests

7 participants