mdn / yari

The platform code behind MDN Web Docs
Mozilla Public License 2.0
1.19k stars 501 forks source link

Add "deprecated" banner based on BCD front matter (and fall back to tag) #3929

Open wbamberg opened 3 years ago

wbamberg commented 3 years ago

(@peterbe asked me to file this, so here goes :) see also https://github.com/mdn/yari/discussions/2905 and https://github.com/mdn/yari/discussions/2905#discussioncomment-618185 in particular)

At the moment we signal deprecation in various ways:

So that's drawing on three different sources to know whether a thing is deprecated. It would be good to rationalize this. In general I think BCD is better-maintained than tags or the manually added macros. Recently we have started adding the BCD query into front matter and extending Yari to build the compat table and the spec table from that data, if it is present. So why not extend this further, and make Yari generate the "Deprecated" banner based on BCD?

The basic plan looks like:

  1. When Yari builds the page it looks for the browser-compat front matter key.
    • If browser-compat is there, and indicates "deprecated", it adds the "Deprecated" banner.
    • If browser-compat is not there, Yari checks the page tags for "deprecated". If the page is tagged "deprecated", Yari adds the "Deprecated" banner.
  2. Change the {{Deprecated_header}} macro to make it a no-op if the browser-compat front matter is present.
  3. Add the BCD front matter property to as many pages as is appropriate.
  4. Once we've finished adding the BCD front matter property, retire the {{Deprecated_header}} macro.

This issue is for the first two of these. The third can happen in parallel.

This plan keeps the "deprecated" tag around indefinitely, but only as a backup for pages that don't have BCD but might still want to indicate deprecation.

@hamishwillee , @ddbeck , @Elchi3 , @Rumyra, @teoli2003 you might be interested in this or even better you might see fatal problems with it!

See also:

hamishwillee commented 3 years ago

Yes to all of the above. It makes sense. This is a better specced duplicate of what I posted in https://github.com/mdn/yari/issues/3818

The things I'd add to this:

ddbeck commented 3 years ago

This plan works for me. A couple of additional notes:

When Yari builds the page it looks for the browser-compat front matter key

I read this and thought there were three distinct states:

  1. Compat-driven state: browser-compat is set, valid, and complete (i.e., there's a front matter string, it refers to a valid feature, and the feature contains the status data)
  2. An error state: browser-compat is set to an invalid feature identifier
  3. Fallback state: browser-compat is not set, or the data lacks status.deprecated (e.g., WebExtensions data doesn't require the status block), so use the backup tag (if applicable)

Not sure what should happen in that second state. Feels sort of wrong to use the backup tag in that case, which feels more broken than intentional

It isn't just Deprecated - the same things apply to Experimental and Non-Standard (note the macro for Experimental has a different name).

I'm not enthusiastic about doing all of this at once (or at least, following the same pattern). I'm skeptical that the data quality for experimental is as good as deprecated (see https://github.com/mdn/browser-compat-data/pull/9933 for more details). Non-standard has a slightly different story which is, more or less: prefer using spec_url to drive this instead.

peterbe commented 3 years ago

It isn't just Deprecated - the same things apply to Experimental and Non-Standard (note the macro for Experimental has a different name).

I'm not enthusiastic about doing all of this at once (or at least, following the same pattern). I'm skeptical that the data quality for experimental is as good as deprecated (see mdn/browser-compat-data#9933 for more details). Non-standard has a slightly different story which is, more or less: prefer using spec_url to drive this instead.

I agree with both "sentiments" here. I think we should consider making "deprecated" a value rather than a key. Each document has an array of "sections". Most of them are "prose" but there's special types like browser_compatibility and specifications too. So I think we should consider adding another one of those and then the Yari document renderer doesn't need to make a special case. E.g.

    "body": [
      {
        "type": "status",
        "value": {
          "id": "deprecated"
        }
      },
      {
        "type": "prose",
        "value": {
          "id": null,
          "title": null,
          "isH3": false,
          "content": "<p><strong><code>BigInt</code></strong> is a built-in object whose constructor returns a....</p>"
        }
      },
      {
        "type": "prose",
        "value": {
          "id": "Description",
          "title": "Description",
          "isH3": false,
          "content": "<p>...</p>

That way it'll be treated as just another "section" and it'll have a so-called "ingredient" component that knows how to render that.

With that out of the way, I think we should NOT inject things into the document that we don't know for certain. E.g.

      {
        "type": "status",
        "value": {
          "id": "experimental"
        }
      },

...should not appear just because the doc happens to have the Experimental tag. At least not until we have a confident way to determine that fact.

peterbe commented 3 years ago

I'll need to sleep on it a bit but I like the idea that we can ask the document, in multiple ways "Are you deprecated?" and it's all the same independent of the usage (e.g. sidebars vs. macro links vs. rendering the doc). And because it could be costly to query this fact repeatedly, we need to implement it in a way that it's overly expensive to find this out over and over in places like the sidebars.

Rumyra commented 3 years ago

Thoughts:

hamishwillee commented 3 years ago

Yes to ^^^ and @Rumyra summary - in particular support the idea of some kind of combined status.

Further thoughts

wbamberg commented 3 years ago

One comment on this bit, that I forgot:

I'll need to sleep on it a bit but I like the idea that we can ask the document, in multiple ways "Are you deprecated?" and it's all the same independent of the usage (e.g. sidebars vs. macro links vs. rendering the doc).

I don't think we should be adding little deprecated icons, or whatever, on links appearing in content. I think we should definitely add banners, and people seem to like the icons in sidebars, so OK, but I think it would look really distracting to add them whenever we link to a page in normal content.

hamishwillee commented 3 years ago

@wbamberg I see your point re ^^^^ but I think that we would find it better if we were distracted.

The reason I suggested this was that

It would be interesting to be able to review what it might look like, or at least to be able to opt-in or opt-out of auto insertion of the icons (perhaps via macro).