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

How to represent changes/moves on the prototype chain #3463

Closed
foolip opened this issue Feb 15, 2019 · 8 comments · Fixed by #9613
Closed

How to represent changes/moves on the prototype chain #3463

foolip opened this issue Feb 15, 2019 · 8 comments · Fixed by #9613
Labels
question Issues where a question or problem is stated and a discussion is held to gather opinions.

Comments

@foolip
Copy link
Contributor

foolip commented Feb 15, 2019

In 5c29205 a few prototype chain changes were found:

  • prefix is on Node.prototype in Edge 12 and on Attr.prototype + Element.prototype in Edge 13. Attr inherits from Node, so it doesn't matter if you have an attr and want to know if you can use attr.prefix. (This happened because of Make Attr a Node again whatwg/dom#102.)
  • className is on HTMLElement instead of Element in Edge 12. It usually doesn't matter, but it can.
  • HTMLOptionsCollection was added in Edge 13, but most of the code one could write would work just as well in Edge 12 where an HTMLCollection was returned, because they had add and remove on that interfaces instead.

In these cases there's a tradeoff between technical accuracy and providing the "will it work" answer that is the main use case for the data. Even with accuracy + notes, a web developer might have to read through an explanation of something that just doesn't matter, and possibly be left in doubt if they don't know the whole Node hierarchy.

@Elchi3 @jpmedley please advise.

@Elchi3 Elchi3 added the question Issues where a question or problem is stated and a discussion is held to gather opinions. label Feb 18, 2019
@foolip
Copy link
Contributor Author

foolip commented Feb 19, 2019

There's another tricky case in #3475 (comment). The XMLHttpRequestEventTarget interfaces was exposed first in Edge 18, but https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6651 shows that the onload member, and probably all the others, were available in Edge 15 and probably Edge 12.

The XMLHttpRequestEventTarget interfaces is mostly a spec authoring convenience, although unlike mixins it is observable. But what mostly matters to web developers is if the concrete instances have the members in question, not which prototype they are on.

@Elchi3
Copy link
Member

Elchi3 commented Feb 19, 2019

We have XMLHttpRequestEventTarget in BCD and on MDN: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequestEventTarget#Browser_compatibility.

I agree these prototypes aren't really that important to web developers, although this one is observable. I hope we could find a convention on how to deal with these situations for both BCD and MDN docs. Right now this is really confusing for both the compat data and web devs reading the docs, I suppose.

Is there a way to determine from webidl which interfaces are just conveniences, which are mixins, which are observable, etc? If we could derive a convention from webidl that would be good rather that making an editorial call on "technical accuracy from specs" vs. "what web developers interact with" for each case. These subjective calls make our API reference docs (and compat data) rather unstructured and hard to read imo. If there is no way of determining this, we need to at least get more familiarity with WebIDL so that tech writers can make better calls here.
Also, I think, exactly this is what differentiates documentation from specifications, so it is important to get this right.

@foolip
Copy link
Contributor Author

foolip commented Feb 19, 2019

Is there a way to determine from webidl which interfaces are just conveniences, which are mixins, which are observable, etc?

Yes, the old style of using [NoInterfaceObject] B and A implements B is gone and mixins or partial interfaces are now used for the spec author convenience cases. https://github.com/foolip/mdn-bcd-updater/blob/master/idl.js has code that merges those into the concrete interfaces.

XMLHttpRequestEventTarget is different though, and more like the 3 cases I filed this issue for. The interface is real, it appears on the prototype chain if implemented, and one could fill out https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequestEventTarget#Browser_compatibility based on whether that change has been made.

Right now I don't think inherited members are represented at all in BCD, but if we do #3441 we would have the challenge of how to represent that the class (interface) hierarchy is different in some browsers. I see two options:

  • Make inheritance its own (probably hidden) entry in the data, so that one could say that Edge N-M had a non-standard inheritance for XMLHttpRequest, and also add onload and other members as non-standard members wherever they actually were.
  • Simply assume the hierarchy from Web IDL, and where that leads to incorrect conclusions about what is supported on which instances, allow the support status for those to be overridden on those interfaces, with notes. That would mean saying that XMLHttpRequestEventTarget wasn't in Edge 12-17 but adding its members on XMLHttpRequest and XMLHttpRequestUpload for the same range.

Either approach adds complexity. As with the original cases in this issue, I'd worry about any solution surfacing irrelevant information to users. This is especially true when XMLHttpRequest and XMLHttpRequestUpload are the only concrete instances you can have, so that it's only if you fiddle with the prototypes that it makes a difference.

@foolip
Copy link
Contributor Author

foolip commented Oct 1, 2020

We've run into this in #6789 too. At first there was just AudioContext, and later BaseAudioContext was added as a common parent interface for both AudioContext and the new OfflineAudioContext. Most members of AudioContext were moved to BaseAudioContext, and BCD fortunately doesn't duplicate most members across the two.

This might mean that there needs to be a wrinkle in the data: that api.BaseAudioContext.createBuffer is said to be added long before api.BaseAudioContext is, because that's the only way to represent that an AudioContext instance would have the createBuffer method available, just not via the same prototype.

@foolip
Copy link
Contributor Author

foolip commented Nov 27, 2020

In #7517 I've suggested how to resolve a subset of these issues.

@foolip
Copy link
Contributor Author

foolip commented Mar 22, 2021

I've recently sorted out two cases where an abstract interface was introduced on the prototype and some members moved up:

The solution was to treat the interface as partial_implementation with notes like "The SVGGeometryElement interface itself is not present, but some of its members are available on the SVGPathElement interface.".

The individual members may also need partial_implementation and notes for some ranges if the set of concrete interfaces they're supported on was expanded, as was the case with SVGGeometryElement.

@foolip
Copy link
Contributor Author

foolip commented Mar 22, 2021

I think that AudioScheduledSourceNode can be handled in the same way.

@jpmedley
Copy link
Contributor

jpmedley commented Mar 23, 2021 via email

foolip added a commit to foolip/browser-compat-data that referenced this issue Mar 25, 2021
foolip added a commit to foolip/browser-compat-data that referenced this issue May 20, 2021
jpmedley added a commit that referenced this issue May 21, 2021
* Add a guideline for APIs moved on the prototype chain

Fixes #3463.

* Split into principle+examples

* Fix lint

* + → and

* Update docs/data-guidelines.md

Co-authored-by: Joe Medley <jmedley@google.com>

Co-authored-by: Joe Medley <jmedley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues where a question or problem is stated and a discussion is held to gather opinions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@Elchi3 @foolip @jpmedley and others