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.

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

Question about Interaction to Next Paint (INP) implementation #194

Closed
SuperOleg39 opened this issue Jun 3, 2022 · 20 comments
Closed

Question about Interaction to Next Paint (INP) implementation #194

SuperOleg39 opened this issue Jun 3, 2022 · 20 comments

Comments

@SuperOleg39
Copy link
Contributor

Hello!

Do you have a plans for Interaction to Next Paint (INP) metric implementation?
Looks like implementation can be non-trivial, according to web-vitals package implementation.

I am interested in your opinion on this metric, as it will affect the size of the package badly, but it seems that this metric still have to start collecting in the future.

@SuperOleg39
Copy link
Contributor Author

@Zizzamia hello! What do you think?

@Zizzamia
Copy link
Owner

Zizzamia commented Sep 1, 2022

Ciao @SuperOleg39, thank you for the ping and sorry for the late reply.

Something I am been thinking for quite a while if it's time to make Perfume.js a superset of the Web Vitals package, by importing the package into Perfume and add some extra functionality on top of it.

This will help stay always align with Google updates, and also keep the extra features that Perfume wants to experiment and offers.

What's your thoughts on this?

@SuperOleg39
Copy link
Contributor Author

SuperOleg39 commented Sep 1, 2022

Hi! Thanks for the answer)

I see only one problem about web-vitals integration - result package size, because at this moment web-vitals have 2.5kb gzip size, аnd perfume.js is 2.3kb gzip (Bundlephobia info).

What do you think about size-limit integration before?

@SuperOleg39
Copy link
Contributor Author

@Zizzamia another reason to add web-vitals - https://github.com/GoogleChrome/web-vitals/blob/main/CHANGELOG.md#v300-2022-08-24

"Report TTFB after a bfcache restore" and new "attribution build" looks like very important and useful.

@Zizzamia
Copy link
Owner

Yeah, I think Perfume.js is better suited to be a superset of web-vitals going forward. Going to make the change pretty soon. Just need to find a few hours to make the switch.

Recently I got distracted on building a Performance library for React Native. But now I am back with more coding time on my hands.

@Zizzamia
Copy link
Owner

Zizzamia commented Nov 6, 2022

Alright, now Perfume.js is a super set of Web Vitals, starting from v8.0.0.

Please, if you see anything that is not working let me know and I will take care of it.

@Zizzamia Zizzamia closed this as completed Nov 6, 2022
@SuperOleg39
Copy link
Contributor Author

Thank you! Will try to integrate soon.

@SuperOleg39
Copy link
Contributor Author

Only one problem, now perfume is 5kb gzip.

But I think than lazy loading solves this problem partially.

@SuperOleg39
Copy link
Contributor Author

Looks like First Paint is unsupported in web-vitals

@SuperOleg39
Copy link
Contributor Author

SuperOleg39 commented Dec 5, 2022

About Cumulative Layout Shift, there is a big change, when CLS fired after page visibility changed to hidden.

It is mean than we can send this metrics only when user leave/hide page twice, because if user will just close page after session, pagehide with sendBeacon will be sended before visibilitychange event (whatwg/html#3957), and no information about CLS.

Maybe reportAllChanges for CLS will be a good default? GoogleChrome/web-vitals#283

Upd. also ask question in web-vitals repo - GoogleChrome/web-vitals#180 (comment)

@SuperOleg39
Copy link
Contributor Author

SuperOleg39 commented Dec 7, 2022

About Cumulative Layout Shift, there is a big change, when CLS fired after page visibility changed to hidden.

The same behaviour with Total Blocking Time

For now it is impossible to collect this metrics when user closing page, if batching is used https://github.com/GoogleChrome/web-vitals#batch-multiple-reports-together

I will try to make a MR with optional reportAllChanges integration`, and looks like we need the same behaviour for TBT

@SuperOleg39
Copy link
Contributor Author

@Zizzamia maybe we start with this changes - https://github.com/Zizzamia/perfume.js/pull/208/files

About TBT, looks like it fired 10+ seconds after FID (expect 5 second because of https://zizzamia.github.io/perfume/#/total-blocking-time/) - so nothing to change here)

@Zizzamia
Copy link
Owner

You right! The fact Perfume does not handle on his on Change Visibility, it does introduce a regression. Which is not good, and makes the data quite unuseful.

I am going to introduce back some fo the code related to Change Visibility that we used to have, and release a new version by the end of the year.

Thank you for keep trying Perfume.js!!!!
Making Perfume.js a superset of web-vitals will require some extra love to make it right. ✨

@SuperOleg39
Copy link
Contributor Author

Will waiting for GoogleChrome/web-vitals#180 (comment)

@Zizzamia
Copy link
Owner

@SuperOleg39 your work in testing both libraries is just the best OSS Christmas present. Thank you man.

I reverted some of the changes I initially made, and trying to recover the initial value of Perfume.js but still keeping the library as superset of web vitals.

https://github.com/Zizzamia/perfume.js/releases/tag/v8.1.6

Let me know if looks better on your side, and love your testing in making Perfume.js better. I will make sure to prioritize your advice going forward.

@SuperOleg39
Copy link
Contributor Author

Thank you for all this work!)

Test GoogleChrome/web-vitals#180 PR with perfume.js@8.2.0 and reportAllChanges for CLS and INP - this metrics works perfect for my case!

Have only trouble with TBT - looks like this metric stop reported?

@SuperOleg39
Copy link
Contributor Author

Looks like TBT not reported, when:

  • page loaded
  • some action, FID is reported
  • immediately leave page (not wait for 10 second for TBT report)
  • back to the page again
  • TBT will never be reported

@SuperOleg39
Copy link
Contributor Author

@Zizzamia
Copy link
Owner

Yeah the case you described is expected!
That's because the reporting to be accurate Perfume needs to wait the 10s to be over.

Glad Perfume, now works well for you.
Please let me know if there is anything I can help you with more.

@SuperOleg39
Copy link
Contributor Author

Thanks for the explanation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants