-
Notifications
You must be signed in to change notification settings - Fork 194
Pin paint timing and add missing keys #3398
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
Conversation
|
PR to fix BCD: mdn/browser-compat-data#28053 I'm not sure we should add Also First-Contentful-Paint (FCP) is Baseline Widely available, but First Paint (FP) is Chrome only and so isn't and the PerformancePaintTiming includes both (as well as these new additions). So does that change anything? Or is it fine to say the API is Baseline Widely available, even if some feature of it aren't? Isn't Baseline supposed to remove that indecision for developers? |
|
This is what this piece of code is trying to do: compute_from:
- api.PerformancePaintTiming
- api.PerformancePaintTiming.first-contentful-paintIt's making the overall status of the entire feature match the status of these 2 BCD keys, ignoring the other keys. The other keys are still part of this feature because they logically belong to it, and it doesn't seem like creating separate features for these other keys is useful. But, perhaps it is? Let me know if the ignored BCD keys can be seen as standalone features which developers might use for separate use cases. Regarding FP vs. FCP, I read a couple of things:
That's why I kept the |
|
Ah OK. In that case I'm cool. Thanks for confirming. I think FCP are what most people want out of this API. FP and the PaintTiming Mixin are optional extras as you say. |
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 think paintTime and presentationTime should be a separate feature.
There appear to be four keys: https://github.com/web-platform-dx/web-features/blob/main/features/draft/spec/paint-timing.yml
It's in Firefox, and Safari seems to be implementing it. It's all a bit confusing, I found this table to be useful w3c/largest-contentful-paint#145 (comment)
|
I think it remains to be seen if Safari will implement So I don't think it's really that valuable as a separate feature. Especially since this concept will also be used for LCP and INP (so is that 3 new features, or all one new feature for all 3)? Ultimately what developers want is FCP, LCP, and INP and this is a subtle extra feature to explain why these are slightly different in different browsers. Assuming that Safari does not implement |
|
Ok, so what's clear is that we should not add |
I think we're on the same page. All I'm saying is that having this defined as its own feature will not regress the "Baseline" status of this feature ("paint-timing") as it will be tracked entirely separately as a new thing where it probably makes sense given different implementation status and goals. |
Yes, but I think in that case I would leave these two keys alone and not add them here. Keep them in draft and make the call about them at a later stage |
|
Makes sense to me. Removed! |
|
If we are adding a new feature it should probably be called "Paint Timing Mixin" as that's what defined these two time stamps. It's maybe a little weird that it's technically part of the Paint Timing Spec, but a separate feature, then then that also reflects the reality that it was created later. I'm just not sure how much the average developer cares about this to be honest. And if they do, they probably actually care who implements Unless we have two new features ("Paint Timing Mixin Paint Time" and "Paint Timing Mixin Presentation Time") but getting very in the weeds then... |
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.
LGTM. BCD should release today, so you could decide to leave this PR open and refresh the dist file once web-features upgraded, or you merge now and you will observe the baseline status change in the dependabot PR.
The following BCD keys are missing from the PerformancePaintTiming API feature:
The toJSON one is easy to add, as it's already supported across the board.
The other ones seem to be newer additions, which only work in Firefox. I decided to add them to the existing feature instead of splitting them out onto their own feature, mostly because I don't know if they are seen as a separate enough thing.
I introduced a
compute_fromstatement to pin the feature to the keys that make the most sense to me.This API has been working for a long time already, and my feeling is developers use it to track the first-contentful-paint entry, which, according to my tests, works fine in all browsers.
BCD seems outdated, and @tunetheweb is fixing it. See https://bsky.app/profile/tunetheweb.com/post/3m22jwkg5hs2y
So, to sum up, with this PR, and with the BCD fix, the PerformancePaintTiming API feature will be Baseline Widely Available, matching the corresponding MDN Baseline banner.