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

Aggregation refactor #1463

Closed
paulirish opened this issue Jan 13, 2017 · 3 comments
Closed

Aggregation refactor #1463

paulirish opened this issue Jan 13, 2017 · 3 comments

Comments

@paulirish
Copy link
Member

paulirish commented Jan 13, 2017

I'm currently working on a change so you can easily select what sort of audits you want to run.
In reality this means need to build a full config that includes all these things:

passes, gatherers per pass, audit list, aggregations (both parent and child aggregations), and the audit list within each of those aggregations

There's a lot of relationships here to juggle and generally it works, but one particular sore spot is "aggregations." What are aggregations anyways? :)

This all led to an idea about a possible refactor..


I have a proposal for adjusting our config/default.json, which will have implications for the pipeline from auditResults => report.

Basically, I'm interested in replacing our big aggregations blob with something like the following:

  • reportCategories array. These are basically our parent aggregations like "PWA", "Fancier Stuff" etc. It's very presentational, and how a visual report will be generated with some semblance of order.
  • auditGroups array. The meat of our aggregations, but it's a flat list with no nesting. so "App can load on offline/flaky connections" is a sibling to "Using modern protocols" and "Page load performance is fast".
  • auditGroupTags object. Describes the few tags that are applied to each auditGroup. These will be used for users configuring what they want to evaluate on a given run.

Each auditGroup (née aggregation) gains an id property, a reportCategory property, and a groupTags one. They don't have any more items property which has 1 or more children, which a lot of code has special handling for. yay. :)

Here's an excerpt of a revised json around the aggregations part:

  ], // end of audits array

  "reportCategories": [
    {
      "name": "Progressive Web App",
      "description": "These audits validate the aspects of a Progressive Web App.",
      "id": "pwa_category",
      "scored": true
    }, {
      "name": "Fancier stuff",
      "description": "A list of newer features that you could be using in your app. These audits do not affect your score and are just suggestions.",
      "id": "fancy_bp_category",
      "scored": false
    }, {
      "name": "Performance Metrics",
      "description": "These encapsulate your app's performance.",
      "id": "perf_diagnostics_category",
      "scored": false
    } // and "Best Practices"..
  ],

  "auditGroupTags": {
    "pwa": "Progressive Web App audits",
    "perf": "Performance metrics & diagnostics",
    "best_practices": "Developer best practices"
  },

  "auditGroups": [
  {
    "name": "New JavaScript features",
    "id": "fancy_best_practices",
    "reportCategory": "fancy_bp_category",
    "groupTags": ["best_practices"],
    "audits": {
      "no-datenow": {
        "expectedValue": false
      },
      "no-console-time": {
        "expectedValue": false
      }
    }
  }, {
    "name": "App can load on offline/flaky connections",
    "description": "Ensuring your web app can respond when the network connection is unavailable or flaky is critical to providing your users a good experience. This is achieved through use of a [Service Worker](https://developers.google.com/web/fundamentals/primers/service-worker/).",
    "id": "offline",
    "reportCategory": "pwa_category",
    "groupTags": ["pwa"],
    "audits": {
      "service-worker": {
        "expectedValue": true,
        "weight": 1
      },
      "works-offline": {
        "expectedValue": true,
        "weight": 1
      }
    }
  }, {
    "name": "Page load performance is fast",
    "description": "Users notice if sites and apps don't perform well. These top-level metrics capture the most important perceived performance concerns.",
    "id": "perf_metrics",
    "reportCategory": "pwa_category",
    "groupTags": ["pwa", "perf"],
    "audits": {
      "first-meaningful-paint": {
        "expectedValue": 100,
        "weight": 1
      },
      "speed-index-metric": {
        "expectedValue": 100,
        "weight": 1
      },
      "estimated-input-latency": {
        "expectedValue": 100,
        "weight": 1
      },
      "time-to-interactive": {
        "expectedValue": 100,
        "weight": 1
      },
      "scrolling-60fps": {
        "expectedValue": true,
        "weight": 0,
        "comingSoon": true,
        "description": "Content scrolls at 60fps",
        "category": "UX"
      },
      "touch-150ms": {
        "expectedValue": true,
          "weight": 0,
          "comingSoon": true,
          "description": "Touch input gets a response in < 150ms",
          "category": "UX"
        },
        "fmp-no-jank": {
          "expectedValue": true,
          "weight": 0,
          "comingSoon": true,
          "description": "App is interactive without jank after the first meaningful paint",
          "category": "UX"
        }
      }
    }, {
    // .. the rest of the auditGroups ....

(Side note: we can now nuke categorizable (today) as it only was there for the "toggle to view report by technology vs user feature".)


I think this approach would really help everyone understand the code quite a bit better. But curious if others think it improves clarity.

WDYT?

@patrickhulce
Copy link
Collaborator

SG! While we're at it can we get rid of all the expectedValues that don't do anything for unscored audits :) Related but perhaps discussion on another thread is more appropriate, I think it might be worth revisiting some audits that are currently pass/fail that could become scored.

@paulirish
Copy link
Member Author

+1 to all that yeah.

@brendankenny
Copy link
Member

categories!

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

No branches or pull requests

3 participants