w3c / performance-timeline

Performance Timeline
https://w3c.github.io/performance-timeline/
Other
111 stars 27 forks source link

Add a note regarding feature detection options for new entry types #64

Closed yoavweiss closed 7 years ago

yoavweiss commented 7 years ago

While reviewing a LongTask implementation patch I noticed that there's no way for developers to detect browser support for new entry types:

I think we should change that so that at least one but ideally both throw or return an error when called with an unsupported entry type.

/cc @spanicker @igrigorik

igrigorik commented 7 years ago

This would result in not so great developer ergonomics..

observer.observe({entryTypes: ['resource', 'mark', 'foo', 'bar', 'other']});
  1. If we throw a blanket exception, the developer doesn't know which is not supported. It also means, that if one browser supports something that others don't, developers are forced to write a lot of fallback boilerplate to catch and resubscribe. A plausible side effect is that it'll force developers to create N subscribers, one for each type.
  2. If we do allow developers to create own entries (https://github.com/w3c/charter-webperf/issues/28) with custom names, then throwing exceptions doesn't make sense either.

Personally, I think we want to keep current behavior for PerfObserver.

For getEntriesByType... We could, in theory, throw if you provide an unknown type. That said, even there, (2) makes things tricky.

yoavweiss commented 7 years ago

I guess that before we'll discuss the "how", we need to decide on the "what".

Do you agree that current situation, where feature detection is not possible, is problematic? e.g. developers using long task observers would also have to continue polling the main thread's availability using timeouts, since they do not know they would get long task entries.

igrigorik commented 7 years ago

Supporting "tell me what types of performance metrics you support" seems like a reasonable and useful use case, yes. FWIW, we had a related discussion here: https://github.com/w3c/performance-timeline/pull/37.

As one idea, we could expose a new attribute on performance interface that returns the list of supported entry types? Curious to hear @toddreifsteck's thoughts on this one..

yoavweiss commented 7 years ago

In general the supports() pattern is frowned upon, when there are alternatives (even if that's where we landed for relList and other DomTokenLists).

igrigorik commented 7 years ago

Frowned upon because..? Probing for each and every type + catching exceptions doesn't seem very developer friendly.

spanicker commented 7 years ago

I disagree with "developers using long task observers would also have to continue polling the main thread's availability using timeouts ..." I don't expect this to be a major problem for longtasks, as they will generally be handled in a dedicated PerformanceObserver, and observe() will throw right away, if unsupported.

spanicker commented 7 years ago

That said I fully support revisiting the observe() interface ( supports 1 argument that is a list of entryTypes). Another major limitation of this interface IMO is the inability to provide any additional arguments to observer. For eg. observe() on longtasks might want to indicate the threshold, observe() on hero-element-timing might want to indicate elements of interest etc.

yoavweiss commented 7 years ago

It seems like there is a way to currently feature detect support for these features, using the presence of e.g. PerformanceEntry, PerformanceLongTaskTiming, etc.

Leaving this open as we probably want to add a note to that effect. I'll change the issue title