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

API structure conventions: inheritance #1006

Closed
Elchi3 opened this issue Jan 6, 2021 · 10 comments
Closed

API structure conventions: inheritance #1006

Elchi3 opened this issue Jan 6, 2021 · 10 comments

Comments

@Elchi3
Copy link
Member

Elchi3 commented Jan 6, 2021

We probably discussed this already somewhere but I'm opening a new issue here.
(I'm also thinking about mix-ins but I figured it might be easier to make a call on simple inheritance structures first.)

I'm picking a random API that has inheritance. It is DOM Range.

interface AbstractRange
interface Range : AbstractRange
interface StaticRange : AbstractRange

So, here we have two constructible classes Range() and StaticRange() and we have a class called AbstractRange which both inherit from.

MDN documents AbstractRange here:

MDN documents Range here:

MDN documents StaticRange here:

Now, MDN documents Range.collapsed and StaticRange.collapsed along with AbstractRange.collapse:

(and this is true for all AbstractRange members, you find them all also under Range and StaticRange).

Observations:

  • There are 3 documents describing the same thing basically.
  • The 3 documents could be describe the 3 different things way better actually (with their specific compat data, or specific code example etc), but this isn't really happening.
  • Only the Range variant manages to provide a code example. StaticRange could have a code example and for AbstractRange you would probably need to refer to Range and StaticRange for practical code.
  • The compat data for AbstractRange is weird and doesn't make sense to me as is.

So, what is the convention here? I think there are options:

  1. Document all abstract and concrete classes plus all their members (seems like this is what is done currently)
  2. Only create an abstract class main page and omit abstract members pages. Document all concrete classes with pages for all members.
  3. Document abstract class and plus all abstract member pages. Only document concrete classes with their specific members and refer to abstract members instead of creating additional pages for them.
@sideshowbarker
Copy link
Collaborator

  1. Only create an abstract class main page and omit abstract members pages. Document all concrete classes with pages for all members.

As far as what is best for developer, I think that is — #2. I believe that’s best because developers don’t actually care about abstract classes, since those aren’t actually exposed/observable.

So I think when developers developers go to look at the MDN article for a particular interface/object, they expect to find is all the exposed members of that interface/object documented/linked to in that one MDN article. They don’t want to be forced to go read yet another article to get the info they need.

Redundancy is the obvious downside of optimizing for the developer reader/user experience is that way. We don’t want to maintain the same information in multiple places.

But we have a mechanism in the Yari backend for doing transclusion. So maybe this is an opportunity to explore using it to solve this long-standing problem. And if the existing Yari transclusion mechanism isn’t sufficient for solving this problem, then maybe we would need to consider how to refine/improve it to handle this.

@chrisdavidmills
Copy link
Contributor

But we have a mechanism in the Yari backend for doing transclusion. So maybe this is an opportunity to explore using it to solve this long-standing problem. And if the existing Yari transclusion mechanism isn’t sufficient for solving this problem, then maybe we would need to consider how to refine/improve it to handle this.

Yes. #2 is the best solution for now, but redundancy is a problem. To get around this, I think we need a combination of separate pages with bits transcluded from a core abstract page — some bits will need to be the same, and some bits will need to be diffferent. So for example:

  • AbstractRange — simple page that just says something like "AbstractRange is a mixin; its features are implemented on Range and StaticRange", with links to those two.

    • AbstractRange.collapsed — opening paragraph saying that this is implemented as Range.collapsed and StaticRange.collaposed, with links to there. Syntax section.
    • etc.
  • Range — interface page as normal.

    • Range.collapsed — opening paragraph as normal, Syntax section transcluded from AbstractRange.collapsed section, examples section, specifications section (including definition of Range and AbstractRange.collapsed, as necessary?), BCD section as normal, See also section as normal.
    • etc.
  • StaticRange — interface page as normal.

    • StaticRange.collapsed — opening paragraph as normal, Syntax section transcluded from AbstractRange.collapsed section, examples section, specifications section (including definition of StaticRange and AbstractRange.collapsed, as necessary?), BCD section as normal, See also section as normal.
    • etc.

So, after writing all that out, I am thinking, is it even worth having the AbstractRange documents created separately, when so much of the pages for the features that implement from there need their own distinct separate sections anyway? You think a lot of it is going to be the same, but really it is just similar but not the same?

In which case this would mean a fourth option to explore — "Document all and concrete classes plus all their members, and get rid of the abstract class/member pages".

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 7, 2021

Thanks for your comments, I'm also leaning towards #2 (or even 4).

So, after writing all that out, I am thinking, is it even worth having the AbstractRange documents created separately, when so much of the pages for the features that implement from there need their own distinct separate sections anyway? You think a lot of it is going to be the same, but really it is just similar but not the same?

Right, this is often the main reason I'm not a big fan of transclusion. You'd have abstract content transcluded when you actually would like to provide specific syntax sections, code examples, etc.

In which case this would mean a fourth option to explore — "Document all and concrete classes plus all their members, and get rid of the abstract class/member pages".

This is interesting. I wonder how much interest MDN readers have about abstract classes (or mixins) at all. If there isn't a need to know this then yes, MDN documentation could actually leave this detail out and readers can consult the specification and WebIDL to learn about Web API classes' composition.

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 7, 2021

I'm going to look around a bit in Web APIs and provide a second (and third) example here to see if our current thoughts apply for other classes that do inheritance as well.

@chrisdavidmills
Copy link
Contributor

This is interesting. I wonder how much interest MDN readers have about abstract classes (or mixins) at all.

I really doubt there is much interest here. On the concrete feature pages, we can include notes to say that "these features are defined on the blah blah mixin", plus link to the spec in the Specifications table. I think that would be enough to satisfy the web archaeologists among us.

I'm going to look around a bit in Web APIs and provide a second (and third) example here to see if our current thoughts apply for other classes that do inheritance as well.

Nice one @Elchi3 — I'm really excited about you looking into this!

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 7, 2021

Okay, discussing a slightly different example: EventTarget.

Non-exhaustive inheritance:

interface Node : EventTarget 
interface AbortSignal : EventTarget
interface AudioNode : EventTarget 
interface XMLHttpRequestEventTarget : EventTarget 
// many more

Basically, EventTarget's addEventListener method and friends are on many classes.

If we think this through in terms of option 2. (or 4.) then we end up with many addEventListener pages like Node.addEventListener, AbortSignal.addEventListener, etc.
Of course, addEventListener will need to documented very similar but the code examples will have different contexts and the compat data is different. However, it is a lot of addEventListener pages potentially. Is this a useful service to MDN readers?

Currently MDN does what I had as option 3. above. See https://developer.mozilla.org/en-US/docs/Web/API/AudioNode#methods

One thing to note here is that EventTarget is constructible which makes it different to the AbstractRange case discussed above. Does this inform the following rule?

  • If a class inherits an abstract/non-constructible class, go with option 2. (or 4.).
  • If a class inherits a constructible class, go with option 3.

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 7, 2021

Does this inform the following rule?

I quickly found another example where I don't think the rule should apply: Node.

interface Document : Node
interface Element : Node
interface Attr : Node 
// ...

We surely don't want to apply option 2. (or 4.) here and create pages for all Node members under the Document, Element, Attr etc. classes. So, how is Node different from AbstractRange? And how can we formulate a rule for this case? I guess it we should go with option 3. but there is no Node constructor that would verify the assumed rule I made above.

Another hint is [Exposed=Window], but I don't understand why AbstractRange has that. If AbstractRange wouldn't have that or even be a mixin it would be clearer to not document it, but for some reason that's not the case here.

@dontcallmedom
Copy link
Contributor

I had the same question as @Elchi3 on why this isn't a mixin - the discussions happened there w3c/staticrange#1 (comment) - from what I can see, it's a combination of alignment with other APIs and the need to reference the common base interface as argument types.

I guess this is relevant here, in the sense that AbstractRange will be encountered in other APIs documented on MDN; but having it simply reference the concrete interfaces seems well-aligned with that.

@Elchi3
Copy link
Member Author

Elchi3 commented Jan 11, 2021

cc'ing @foolip who had thoughts on this in the past. Philip, I'm trying to think about how to best deal with (simple) API inheritance structures on MDN pages (not even mixins yet). Do you have thoughts on the examples above? What would you expect from documentation?

@chrisdavidmills
Copy link
Contributor

@Elchi3 I think it is good that you've brought up common abstract classes that are implemented in many places. You are right that in places like these, we shouldn't repeat the pages endless times for the sake of it.

Maybe we should think about defining the two cases as two different groups — specialized abstract classes, and common abstract utility classes (or somesuchthing)?

The first group would be ones like AbstractRange that aren't implemented in too many places, and are likely to require different examples, etc. These would be dealt with using option 4.

The second group would be ones like EventTarget, which are pretty generic and implemented in lots of places. These would be dealt with using option 3, and then on implementing class pages, we could do what we currently do anyway in many cases, and include some functionality in Yari to include collapsed lists of links to all the stuff that is defined on the abstract classes.

Of course, exactly what rules will put an abstract class in one group or the other is still up for debate ;-)

@Rumyra Rumyra added needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. enhancement and removed needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Jun 7, 2021
@mdn mdn locked and limited conversation to collaborators Jun 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants