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.

Sign up for GitHub

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

Adding subfeatures guidance #17352

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Adding subfeatures guidance #17352

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7795521
Add info about when you add a subfeature
hamishwillee Aug 22, 2022
b163287
Merge branch 'main' into patch-9
hamishwillee Aug 26, 2022
fa6b76f
Update api.md
hamishwillee Aug 26, 2022
1251cd2
Update docs/data-guidelines/api.md
hamishwillee Sep 19, 2022
81b61eb
Merge branch 'main' into patch-9
queengooborg May 11, 2024
aeb546f
Separate subfeature guidelines into its own file
queengooborg May 11, 2024
a6b4524
Update guidelines to better match BCD conventions
queengooborg May 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions docs/data-guidelines/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,49 @@

This file contains guidelines that are specific to the web API features (`api/`).


## 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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Edit the page how you like.

For Web APIs the top level features are individual Web API interfaces and the children of namespace objects like `Intl`.

A possible structure for a Web API is shown below.

```
api // category - always api for Web APIs
interface // top level feature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also globals like fetch().

properties // feature: required
methods // feature: required
parameters // subfeature: compatibility is different
parameter options // subfeature: if compatibility is different
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@hamishwillee hamishwillee Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those definitions are good.


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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also applies to parameter options.


> **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_.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


> **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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


Here is an example scenario:

- An interface is defined in a specification with a method (feature) that has two parameters (subfeatures).
- 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 _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 specification changes to require that the interface now can be used in workers:
hamishwillee marked this conversation as resolved.
Show resolved Hide resolved
- A "supported in workers" subfeature is added to the interface feature because this is a change in compatibility.

> **Note:** If all browsers implemented worker support in their first implementation then no subfeature would be required.
> It is only once a browser does something different that the subfeature is created.

## Constants

Don't include data for constants in BCD. The rationale for not including them is that they're not known to be a source of any compatibility issues.
Expand Down