-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Reduce Performance APIs to a single group #22031
Conversation
a616a98
to
caf4bc3
Compare
caf4bc3
to
2566ccd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So fiddly, this.
"/docs/Web/API/User_timing_API/Using_the_User_Timing_API", | ||
"/docs/Web/API/Element_timing_API", | ||
"/docs/Web/API/Paint_timing_API", | ||
"/docs/Web/API/Largest_Contentful_Paint_API" | ||
], | ||
"interfaces": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that three lists of interfaces need to match:
- the list of interfaces here
- the list in the perf overview page
- the set of interfaces (and subpages) that are calling
APIRef("Performance API")
We have to maintain all three lists manually because our infra is so bad (see also openwebdocs/project#76 (comment)).
I've been through the Perf WG specs and made a list of all the interfaces that seem relevant:
EventCounts
InteractionCounts
LargestContentfulPaint
Performance
PerformanceEntry
PerformanceEventTiming
PerformanceLongTaskTiming
PerformanceMark
PerformanceMeasure
PerformanceNavigation (obsolete)
PerformanceNavigationTiming
PerformanceObserver
PerformanceObserverEntryList
PerformancePaintTiming
PerformanceResourceTiming
PerformanceServerTiming
PerformanceTiming (obsolete)
TaskAttributionTiming
The groupdata entry here does seem to be consistent with the perf overview page, but there a few discrepancies between both of them and my list:
- you are missing
InteractionCounts
. I suspect actually both this andEventCounts
should not get their own pages but should be documented inline. But I think what we do for one we should do for both. - you have
PerformanceElementTiming
, but this is a WICG interface. Still, it is already documented. Not sure what to do here or what our general policy is on WICG specs. I suspect we have a lot of them already. - you are missing
PerformanceNavigation
andPerformanceTiming
Compared with pages that have APIRef("Performance API")
in this PR:
- you have it for the
PerformanceNavigation
andPerformanceTiming
pages - you have it for
DOMHighResTimestamp
, which isn't an interface of course. IMO we should delete this page, per Delete DOMTimestamp and EpochTimestamp #17096, and just have a bit somewhere describing timestamps in the perf API. - you have it for
PerformanceElementTiming
- you are again missing
InteractionCounts
So I think we need to:
- add
PerformanceNavigation
andPerformanceTiming
to groupdata and the perf overview page - get on with deleting
DOMHighResTimestamp
- either add
InteractionCounts
, or removeEventCounts
, probably the latter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- you are missing
InteractionCounts
. I suspect actually both this andEventCounts
should not get their own pages but should be documented inline. But I think what we do for one we should do for both.
So, both are interfaces, and I thought our policy is that we always document interfaces on their own page. BCD will always have data for interfaces, too. The reason that EventCounts is there and InteractionCounts isn't, is that InteractionCounts isn't supported anywhere yet but EventCounts is. (and so there is BCD for EventCounts and not for InteractionCounts).
- you have
PerformanceElementTiming
, but this is a WICG interface. Still, it is already documented. Not sure what to do here or what our general policy is on WICG specs. I suspect we have a lot of them already.
It was already documented and has an implementation, so I think we should care about it.
- you are missing PerformanceNavigation and PerformanceTiming
Both are deprecated. I'm happy to add them, but maybe it would be better to discourage them as much as possible?
get on with deleting DOMHighResTimestamp
Yes, I think we should do this. Need to make sure that performance docs can still refer to this type somewhere, though, I think. Will look into the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- you are missing
InteractionCounts
. I suspect actually both this andEventCounts
should not get their own pages but should be documented inline. But I think what we do for one we should do for both.So, both are interfaces, and I thought our policy is that we always document interfaces on their own page. BCD will always have data for interfaces, too. The reason that EventCounts is there and InteractionCounts isn't, is that InteractionCounts isn't supported anywhere yet but EventCounts is. (and so there is BCD for EventCounts and not for InteractionCounts).
Oh, sorry, I didn't check the support.
- you have
PerformanceElementTiming
, but this is a WICG interface. Still, it is already documented. Not sure what to do here or what our general policy is on WICG specs. I suspect we have a lot of them already.It was already documented and has an implementation, so I think we should care about it.
+1
- you are missing PerformanceNavigation and PerformanceTiming
Both are deprecated. I'm happy to add them, but maybe it would be better to discourage them as much as possible?
I think we should add them. Discouraging comes with deprecation. But we need to be consistent either way - this PR adds {{APIRef("Performance API")}}
to these pages )(https://github.com/mdn/content/pull/22031/files#diff-f75804c2d8e3509e92e7d6ff6c78e1eb8e2eb431ea2140fa0d8314a62c753b58R20), so they should appear in the groupdata entry as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay fair enough, I added them. Do you think I should file a yari issue because it doesn't display the deprecated icon in the sidebar for these two under "Related pages for Performance API" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean for instance in the sidebar at https://pr22031.content.dev.mdn.mozit.cloud/en-US/docs/Web/API/Performance_API/Using_the_Performance_API ?
tbh I think if you actually want it to be fixed, file a PR to do it and I will review it. It looks like DefaultAPISidebar doesn't look at Deprecated etc tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, will do :)
@@ -12,7 +12,7 @@ tags: | |||
browser-compat: api.PerformanceElementTiming | |||
--- | |||
|
|||
{{DefaultAPISidebar("Element Timing")}}{{SeeCompatTable}} | |||
{{DefaultAPISidebar("Performance API")}}{{SeeCompatTable}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually we will delete these other overview pages, won't we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would do that as a follow-up once we have a replacement page to redirect to (we don't have it just yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Description
The Performance API consists of a few specs, however, web developers don't really need to know about this. Important is that the web platform offers a Performance API to measure web application's performance and we should present all the measurement possibilities at a central place where you can learn about all of them and which one is suitable in which situation.
Motivation
openwebdocs/project#62
Additional details
I updated the list of guides in groupData as well, but note that they will all be rewritten and we will have one guide to all Performance APIs.
There is a weird mix of APIRef and DefaultAPISidebar. Maybe we can fix that some other day, though.
Related issues and pull requests
#21984 updates the main Performance API overview page and will be the new main page for all Performance APIs.