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

docs(plugins): add link to field-performance plugin #9051

Merged
merged 1 commit into from
May 24, 2019

Conversation

paulirish
Copy link
Member

Followup to #9049.. linking here will be useful for prospective plugin authors :)


Also...

@alekseykulikov @denar90 I was hoping to get some feedback from you guys on writing a plugin.. It looks like there were some rough edges and I think we could improve docs and perhaps some APIs to make it more straightforward for the next plugin :)

What did you notice? What was annoying? What was hard to figure out? What could we document better? :)

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fantastically written example 👍

@denar90
Copy link
Contributor

denar90 commented May 24, 2019

Hi, thanks, folks :)

We were so happy to have that API and really enjoyed working on the plugin. Other than #9050 #9052 I can add about html rendering.

The first intention was to make performance score representation like it's for "performance" category, but digging into the code we've noticed it's not so easy without patching LH.

There are special rendering for PWA and Performance - https://github.com/GoogleChrome/lighthouse/blob/v5.0.0/lighthouse-core/report/html/renderer/report-renderer.js#L227-L230. Will be nice to accept the custom for other categories.

So then we switched to table view which we found perfect for distribution representation. What we also brainstormed about there, were:

  • style row or table somehow (to underline slow, average, fast categories and their values)
  • accept values as html. I had this crazy idea to render some graph or other visualization via preact and then do render-to-string to pass the result as value (plain html).

One more, mixing static audit meta data with dynamic values. We couldn't add some info about the current page and origin like:

image

@brendankenny
Copy link
Member

fantastically written example 👍

ha, it took me a while to realize you weren't sarcastically referring to @paulirish's one line change here :)

#9051 (comment)

Good feedback, we should capture in some issues in addition to #9053

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 🔗

@brendankenny brendankenny merged commit 4aeb0b3 into master May 24, 2019
@brendankenny brendankenny deleted the paulirish-patch-1 branch May 24, 2019 23:47
@patrickhulce
Copy link
Collaborator

it took me a while to realize you weren't sarcastically referring to @paulirish's one line change here

Haha, yes sorry that was intended as a compliment to @denar90 and @alekseykulikov , no dig on @paulirish :)

@alekseykulikov
Copy link
Contributor

Hi all!

In addition to @denar90's reply, I could list a few minor issues:

  • We use TypeScript with allowJs (similar to your setup), and validation requires devtools-protocol, which is a part of devDependencies of Lighthouse and not installed. It required us to add devtools-protocol to our package.json to bypass the validator.
  • More recommendations regarding scoring performance values would be helpful for plugin authors. We used Lighthouse's Audit.computeLogNormalScore to score FCP/FID (code), and I'm not sure is it part of the stable API or not.
  • I would review the assumption, that plugin can't gather custom data from the page (artifacts). field-performance plugin fetches the data from PSI API, then cache it internally to use in the rest of audits. (code). It's possible to cache internally, but still would be valuable to hook to Lighthouse's gather process. For example, if @patrickhulce would build a third-party plugin, he would need to make another Lighthouse run without all 3rd-party scripts, which is not possible now.

I have some feedback about PSI API/UI, and I will send you @paulirish by email :)

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

Successfully merging this pull request may close these issues.

5 participants