Adding subfeatures guidance #17352

hamishwillee · 2022-08-15T02:42:19Z

The current guidance explains how you add subfeatures, but omits how you decide when a subfeature is required.

My understanding is that you only add subfeatures for compatibility differences. So someone seeing that an interface is supported knows that all the browsers implement that subfeature in the same way in those particular versions.

BUT they don't know exactly what methods, properties etc are exactly implemented from that data. While they might "hope" that everything in the spec is implemented, what they actually know is that they are all the same implementation in those indicated versions.

I wanted to check this because the use case [ImageData] used widely in the doc does not comply with these rules - I'll comment below.

This comes out of the discussion here: #7849 (comment)

FYI @queengooborg @foolip

docs/data-guidelines.md

hamishwillee · 2022-08-15T02:48:32Z

docs/data-guidelines.md

+
+Subfeatures should only be added to indicate a  _difference_ in browser compatibility versions from the parent feature; they should not be added just to indicate that a particular feature exists.
+
+For example, the `ImageData` interface was initially defined with `data`, `height` and `width` properties.


This is not actually true ImageData has these values defined in BCD. We should remove them right @queengooborg ?

FWIW I understand the reason for not documenting "to indicate support", but it does make it hard to know whether you have just omitted something or there is no known compatibility break.

It's actually the opposite: all subfeatures should be added regardless of whether they match the parent version! Each subfeature should be added to BCD, even if the data matches its parent, to help with clarity. Say ImageData has recently added a new format subfeature -- consumers of BCD wouldn't be able to tell whether or not format has been supported if the assumption is that all undefined subfeatures match the parent.

Behavioral differences, on the other hand, are more subjective. If the spec has changed the behavior of a feature and browsers needed to update, then we should add a subfeature for that behavior change.

There are a number of subfeatures we are missing from BCD at the moment, which I'm hoping to fix in the future!

@queengooborg OK. That makes sense. It concerns me that this was not communicated to me. Or the new way of adding bug URLs.

I'll try an update then.

P.S. We could reduce duplication by allowing "support": mirror for the whole compat section of a subfeature implemented at the same versions as its parent.

docs/data-guidelines.md

hamishwillee · 2022-08-15T05:34:13Z

docs/data-guidelines.md

@@ -12,6 +12,17 @@ If it's helpful to understanding the rule, summarize the rationale. Definitely c

 -- END TEMPLATE -->

+
+## Adding subfeatures


@queengooborg @foolip OK, I have reworded this to be the opposite. This says "always add a subfeature for any entity or behaviour where there is an implementation in a browser."

hamishwillee · 2022-08-19T06:30:24Z

docs/data-guidelines.md

+Subfeatures entries are added for all:
+
+- interface methods and properties that have an implementation in any of the supported browsers.
+- ?


Are there an other exceptions?

hamishwillee · 2022-08-19T06:34:32Z

docs/data-guidelines.md

@@ -12,6 +12,45 @@ If it's helpful to understanding the rule, summarize the rationale. Definitely c

 -- END TEMPLATE -->

+
+## Adding features and subfeatures


@queengooborg Updated as per our discussion:

features required for all released web platform features. Term feature is now defined.

subfeatures term now defined

subfeature required for interface methods and properties by default if there is any released implementation.

subfeatures not requried for anything else unless there is a change in version support. This has nothing to do with what the spec is doing and is entirely due to some implementation change.

I think this is at least close, though there might be other exceptions like interface methods and properties. See comment below.

There is also the slight problem of things like HTTP headers which are not necessarily implemented in a browser, and for which the term "supported" is very loose. I think we should get this in and then address that separately.

github-actions · 2022-08-22T20:23:22Z

This pull request has merge conflicts that must be resolved before it can be merged.

foolip · 2022-09-13T15:02:32Z

docs/data-guidelines/api.md

+
+## Adding features and subfeatures
+
+A json file should be created for every _top level feature_ that has a released public implementation on one of the supported set of browsers (and not before).


The "and not before" here should come with a lint I think, and I think running that lint will reveal that more things are removed by this rule than perhaps we really want to see removed.

But if we can sort through that, I think that we should add things to BCD when they're in a beta browser release, to not have to wait until stable before documenting things, if someone is willing to do the work earlier.

foolip · 2022-09-13T15:03:10Z

docs/data-guidelines/api.md

+
+```
+api                           // category - always api for Web APIs
+  interface                   // top level feature


Also globals like fetch().

foolip · 2022-09-13T15:05:09Z

docs/data-guidelines/api.md

+The interface must include subnodes for _all_ its properties and methods (that have an implementation in at least one of the supported browsers), whether or not they were added at the same time as the parent interface, and hence share the same compatibility information.
+
+All other code and behavioral aspects of the interface are given nested nodes **only** if their compatibility is _different_ to that of their parent feature.
+This applies to method arguments, method argument options, and behaviour changes like the addition of support for workers, or the type of return value.


Suggested change

This applies to method arguments, method argument options, and behaviour changes like the addition of support for workers, or the type of return value.

This applies to method parameters, and behaviour changes like the addition of support for workers, or the type of return value.

It also applies to parameter options.

foolip · 2022-09-13T15:05:50Z

docs/data-guidelines/api.md

+    properties               // feature: required
+    methods                  // feature: required
+       parameters             //   subfeature: compatibility is different
+         parameter options    //   subfeature: if compatibility is different


What's this? We don't nest parameters, they have a structure like options_capture_parameter.

foolip · 2022-09-13T15:07:08Z

docs/data-guidelines/api.md

+
+## Adding features and subfeatures
+
+A json file should be created for every _top level feature_ that has a released public implementation on one of the supported set of browsers (and not before).


I don't think we should talk about the file structure here. That's actually very complicated, and better done as a lint I think. Plenty of little edge cases.

OK. Edit the page how you like.

foolip · 2022-09-13T15:08:41Z

docs/data-guidelines/api.md

+    support in workers     // subfeature: if compatibility is different (might also just be on specific methods)
+```
+
+The interface must include subnodes for _all_ its properties and methods (that have an implementation in at least one of the supported browsers), whether or not they were added at the same time as the parent interface, and hence share the same compatibility information.


Suggested change

The interface must include subnodes for _all_ its properties and methods (that have an implementation in at least one of the supported browsers), whether or not they were added at the same time as the parent interface, and hence share the same compatibility information.

The interface must include subfeatures for _all_ its properties and methods (that have an implementation in at least one of the supported browsers), whether or not they were added at the same time as the parent interface, and hence share the same compatibility information.

@foolip Part of the reason for this update was that the term subfeatures was undefined. I thought it just meant a sub node of some other node, but @queengooborg was using it to mean a property or a method below the top level feature.

I preferred my definition because otherwise how do you refer to the parameters of a method? The aren't a subfeature as Vinyl uses it. That's what forced me to use node and subnode. It feels wrong to me, but I'm not the boss of BCD.

@queengooborg can you clarify the definitions you'd like to use?

What we have in BCD is a tree, where some nodes in the tree have __compat. These nodes are called "features" in traverse.js, and I've often called them entries as well.

I think there are two potentially useful concepts:

Any feature that sits below another one in the tree, like "api.AbortController.abort" (because "api.AbortController" is a feature)

Things that are behaviors of another feature and only worth adding if support differs, like "api.fetch.blob_data_support".

I don't know what the arguments are, but my initial preference would be to just call everything features, and talk about "behavioral features" or "subfeatures" where needed to be precise.

I'd like to define the terms as such:

Feature: any object with a __compat statement (ex. api.Foo)

Subfeature: any feature that is a level below another feature (ex. api.Foo.bar)

Behavioral feature: any feature that describes a behavior, parameter, object, etc. (ex. api.Foo.worker_support, or api.Foo.bar.frozen_value)

I think those definitions are good.

foolip · 2022-09-13T15:09:05Z

docs/data-guidelines/api.md

+All other code and behavioral aspects of the interface are given nested nodes **only** if their compatibility is _different_ to that of their parent feature.
+This applies to method arguments, method argument options, and behaviour changes like the addition of support for workers, or the type of return value.
+
+> **Note:** Items that must be given nodes are referred to as "features", while those that are only created when there is a compatibility difference to the parent are called _subfeatures_.


How about not using the word "node" at all but just talk about features and subfeatures?

@foolip See https://github.com/mdn/browser-compat-data/pull/17352/files#r972516073

I would be happy with this if we used feature and subfeature sanely. But we don't. So define it properly - otherwise node and sub node are better.

If I sound frustrated, it is because I am. So much time wasted on this. IMO if we just said "feature" is the top level item and subfeature is any nested item at any nesting level this would be fine. But you'll have to get @queengooborg to agree - I couldn't

Huh? But that's exactly the definition of "subfeature" I was hoping for...

@queengooborg I really hope I haven't been in "violent agreement" with you for most of this discussion :-0.

foolip · 2022-09-13T15:09:37Z

docs/data-guidelines/api.md

+
+> **Note:** Items that must be given nodes are referred to as "features", while those that are only created when there is a compatibility difference to the parent are called _subfeatures_.
+
+> **Tip:** A good rule of thumb is that if a subfeature has the same compatibility information as its parent, then the subfeature is not needed.


This rule of thumb contradicts the very specific (and good!) rules right above.

I don't think so. See the comment above:

Note: Items that must be given nodes are referred to as "features", while those that are only created when there is a compatibility difference to the parent are called subfeatures.

So its a subfeature only if there is a compatibility difference.

docs/data-guidelines/api.md

hamishwillee · 2022-09-16T00:50:18Z

@foolip Thanks for reviewing this.

I'm a bit frustrated with how this ended up - mostly because we previously had no definitions of our terminology (i.e. for subfeature), and now we have ones that I think are annoying.

How about you and Vinyl fight this out and just make the edits you need. I understand the rules now, so not so concerned :-)

foolip · 2022-09-19T13:11:45Z

@hamishwillee I hope we can land on some definitions that make sense to everyone.

@queengooborg It looks like we are invited to duel. Given your beat saber skills I think I'd be quickly shredded if we do it with swords, but let's see if we can work it out in comments instead 😄

hamishwillee · 2022-09-19T23:30:26Z

@foolip Yes, you two must duel; let the gods of clarity prevail.

Apologies that I am taking my grumps out on you two. This seemed like this task should be so easy and it has not been.

I'm OK with everything being a feature, and using "behavioral features" to mean what it means.

The problem is that for this to work we need to explain the conditions when you need to create a sub-node for a new feature -and when you do not. So we create a sub node by default for methods and properties of an interface but we do not for the arguments of a function or constructor.

I wanted to use the name "subfeature" to refer to any subnode of the method. If we can't do that, then find an acceptable substitute you'll be happy with and this should not end up being hard.

queengooborg · 2024-01-07T15:29:07Z

This PR is the oldest PR currently open, and I don't see us coming to a resolution any time soon (this is unfortunately a low priority for us right now). What should we do with this PR?

Elchi3 · 2024-01-17T09:37:57Z

I still want to review this. Recently, there were discussions about features, subfeatures, behavioral features in the context of groups.

caugner

LGTM overall, here are three suggestions to hopefully bring this towards a consensus.

caugner · 2024-10-10T15:56:23Z

docs/data-guidelines/features.md

+> A good rule of thumb is that if a behavioral subfeature has the same compatibility information as its parent, then the subfeature is not needed.
+> Another good rule of thumb is that if the signature of a method changes to introduce new parameters, then all parameters should be documented.
+
+Here is an example scenario:


Keep it simple:

Suggested change

Here is an example scenario:

Example:

caugner · 2024-10-10T16:06:34Z

docs/data-guidelines/features.md

+- An interface is defined in a specification with a method that has two parameters.
+- A supported browser implements the method and both the parameters:
+  - A feature node is created for the method (by default)
+  - Subfeature nodes are not created for the method parameters because they were created in the same version as the methods; there is no compatibility change.
+- Another browser implements the methods, but only one of the parameters.
+  - A subfeature node is added to the method parameter that was supported by _both_ browsers.
+  - Another subfeature node is added to the method parameter that was _not_ supported by the second browser.
+- Yet another browser implements the method with a new parameter.
+  - A method subfeature is added for the new parameter because it was added in a different version than the parent method.
+- The browser changes to require that the interface now can be used in workers:
+
+  - A "supported in workers" subfeature is added to the interface feature because this is a change in compatibility.


Here's how I would restructure the example for better readability:

Suggested change

- An interface is defined in a specification with a method that has two parameters.

- A supported browser implements the method and both the parameters:

- A feature node is created for the method (by default)

- Subfeature nodes are not created for the method parameters because they were created in the same version as the methods; there is no compatibility change.

- Another browser implements the methods, but only one of the parameters.

- A subfeature node is added to the method parameter that was supported by _both_ browsers.

- Another subfeature node is added to the method parameter that was _not_ supported by the second browser.

- Yet another browser implements the method with a new parameter.

- A method subfeature is added for the new parameter because it was added in a different version than the parent method.

- The browser changes to require that the interface now can be used in workers:

- A "supported in workers" subfeature is added to the interface feature because this is a change in compatibility.

Given: A specification defines an interface with a method that has two parameters.

Assume: One supported browser implements the method and both the parameters.

Then:

- We create a feature node for the method (by default)

- We don't create subfeature nodes for the method parameters, because they were created in the same version as the methods; there is no compatibility change.

Assume: Another browser implements the method, but only one of the parameters.

Then:

- We add one subfeature node for the parameter that is supported by _both_ browsers.

- We add another subfeature node for the parameter that is only supported by the first, but _not_ by the second browser.

Assume: Yet another browser implements the method with a new parameter.

Then:

- We add yet another subfeature node for the new parameter, because it was added in a different version than the parent method.

Assume: One of the browsers starts supporting the interface in workers:

Then:

- We add a `worker_support` subfeature (`"Available in workers"`) to the interface (see [Web Workers](./api.md#web-workers-worker_support)).

@caugner I have no problem with this alternative approach, but I think I'd like the change the rule, not that I've been around a while.

The "rule" for API properties and methods is that you always add entries for all that are supported.

The current rule for method arguments (as I understand it and stated there) is that you only document the incompatibility. So the assumption is that if something is not documented then it is compatible.

I'd like to change the rule to "As soon as there is an incompatibility you should document all parameters including those that were in the original version.

The reason is that as it is now there is no way to spot whether something is compatible or has just been omitted.

Upshot, if we know the rules I'm open to any presentation at all.

If this PR explains the rule different than the current status quo, then let's document the status quo first, and then propose the change afterwards. That way we separate concerns and make it easier to move forward.

Anyways, I'll put this on the agenda for the next BCD call.

Well, I wrote it to "test" the rules. So right now it reflects what I think the rules were, but not necessarily what they are.

caugner · 2024-10-10T16:10:21Z

docs/data-guidelines/features.md

+```
+category                // category name (always `api` for API interfaces)
+  interface               // top level feature
+    properties              // subfeature, add if supported
+      property values         // parameter subfeature, add all of them if not all values were supported since property was introduced
+    methods                // subfeature, add if supported
+      parameters              // parameter subfeature, add all of them if not all parameters were supported since method was introduced
+        parameter options       // parameter subfeature, add all of them if not all parameter options were supported since property was introduced
+        parameter type          // parameter subfeature, add all of them if not all parameter types were supported since property was introduced
+    support in workers     // behavioral subfeature, add if compatibility is different (might also just be on specific methods)
+```


Ca we use nested lists here, rather than a code block?

Suggested change

```

category // category name (always `api` for API interfaces)

interface // top level feature

properties // subfeature, add if supported

property values // parameter subfeature, add all of them if not all values were supported since property was introduced

methods // subfeature, add if supported

parameters // parameter subfeature, add all of them if not all parameters were supported since method was introduced

parameter options // parameter subfeature, add all of them if not all parameter options were supported since property was introduced

parameter type // parameter subfeature, add all of them if not all parameter types were supported since property was introduced

support in workers // behavioral subfeature, add if compatibility is different (might also just be on specific methods)

```

- _category_ ← category name (always `api` for API interfaces)

- _interface_ ← top-level feature

- _properties_ ← subfeature, add if supported

- _property values_ ← parameter subfeature, add all of them if not all values were supported since property was introduced

- _methods_ ← subfeature, add if supported

- _parameters_ ← parameter subfeature, add all of them if not all parameters were supported since method was introduced

- _parameter options_ ← parameter subfeature, add all of them if not all parameter options were supported since property was introduced

- _parameter type_ ← parameter subfeature, add all of them if not all parameter types were supported since property was introduced

- _worker support_ ← behavioral subfeature, add if compatibility is different (might also just be on specific methods)

github-actions bot added the docs Issues or pull requests regarding the documentation of this project. label Aug 15, 2022