Skip to content
This repository was archived by the owner on Jan 27, 2019. It is now read-only.

Conversation

@jxnblk
Copy link
Member

@jxnblk jxnblk commented Jul 28, 2015

v2

This is presented as a prototype for discussion. This doesn't have to be the actual v2, but I do like a lot of the things happening here. Would love some feedback and questions.

What

  • Smaller, simplified, and reorganized object
  • Reduce duplication with PostCSS functionality
  • Works as a postcss plugin
  • Remove stats that can be determined in the app/presentation layer
  • Lite option
  • Full property names are used as keys rather than camel casing
  • Uses standard
  • New Helper methods

New returned object

// n = number
// str = string
// [] = array
// prop = CSS property key

{
  size: n,
  gzipSize: n,
  rules: {
    total: n,
    size: {
      graph: [n], // Array of numbers for each rule's size (e.g. number of declarations)
      max: n,
      average: n
    }
  },
  selectors: {
    total: n,
    id: n,              // total number of id selectors
    class: n,           // total number of class selectors
    type: n,            // total number of type selectors (not currently implemented)
    pseudoClass: n,     // total number of pseudo class selectors
    psuedoElement: n,   // total number of pseudo element selectors
    repeated: [str],    // array of repeated selectors
    values: [str],      // array of all selectors
    specificity: {
      max: n
      average: n
    },
    getSpecificityGraph(),
    getRepeatedSelectors(),
    getSortedSpecificity()
  },
  declarations: {
    total: n,
    important: n,       // total number of important declarations
    vendorPrefix: n,    // total number of vendor prefixed declarations
    properties:         // object for each unique property
      prop: [str]       // array of values for each property
    },
    getPropertyResets(),
    getUniquePropertyCount(),
    getPropertyValueCount()
  },
  mediaQueries: {
    total: n,
    unique: n,
    values: [str]       // array of values for media queries (e.g. '(min-width: 40em)')
  }
}

To do

  • Implement lite option
  • Clean up unused modules and/or create new modules
  • Count type selectors
  • Update README
  • Account for missing reset declarations
  • Update/add new tests

Questions

  • Does this organizational scheme make sense?
  • Can/should we handle any shorthand property parsing at the presentation layer?
  • Is there anything else we'd want to parse for media queries?
  • How should the lite option be implemented? Should opts be passed to each module in lib? Or should the modules be split up further?

Usage examples

UPDATE
I've added some helper methods that make the examples below redundant.

Get total number of unique colors

var _ = require('lodash')
var cssstats = require('cssstats')

var stats = cssstats(css)

var uniqueColorsCount = _.uniq(stats.declarations.properties.colors).length

display: none count

var displayNoneCount = stats.declarations.properties.display
  .filter(function (val) {
    return val === 'none'
  })
  .length

Find the selector with the highest specificity

var maxSpecificity = _.max(stats.selectors.specificity.graph)
var index = stats.selectors.specificity.graph.indexOf(maxSpecificity)
var maxSelector = stats.selectors.values[index]

Sort selectors by highest specificity

var ranked = _.zipWith(stats.selectors.values, stats.selectors.specificity.graph, function (a, b) {
    return {
      selector: a,
      specificity: b
    }
  })
  .sort(function (a, b) {
    return b.specificity - a.specificity
  })

Specificity graph

var graph = stats.selectors.specificity.graph

Rule size graph

var graph = stats.rules.size.graph

@HipsterBrown
Copy link

Can you clarify what you mean by "lite option"?

Could organizing the returned object's properties alphabetically be the simplest convention anyone reading through it?

Besides those questions, this looks like a rad update. I love the use case examples and performance optimizations overall. I've rarely seen a better looking PR. ❤️

@jxnblk
Copy link
Member Author

jxnblk commented Jul 28, 2015

@HipsterBrown

  • lite option would mean arbitrarily removing some of the larger objects and arrays (e.g. declarations.properties) to help with keeping the size down and for performance.
  • Sorting the properties object could make sense. Currently it's sorted by source order, which I tend to prefer, but might not matter as much for this.
  • Thanks! 😎

lib/selectors.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

😻

@rafaelrinaldi
Copy link

👏 This is beautiful! I was thinking of improving the return object while working on a project that relies on css-statistics... I think you've found a really great format.

Just one thing though, what if properties like vendorPrefix or important return an array of occurences instead of a number only? This way its easier to spot the ones showing up the most and things like that.

@johno
Copy link
Member

johno commented Jul 28, 2015

This is amazing! Loving how this approach is cleaning up the code so drastically while removing a lot of duplicated return content.

tl;dr

:shipit:


Does this organizational scheme make sense?

Definitely.

Can/should we handle any shorthand property parsing at the presentation layer?

I've been pondering this one a bit, and I think that the presentation layer is likely the best location for this. It feels kinda hacky to add a font-size: 123px declaration in the returned object when it already exists in the font shorthand.

So, it's likely best to expose a function that is contextually aware of something like font-size and the font shorthand. How exactly is the best way to do this, I'm not quite sure.

Is there anything else we'd want to parse for media queries?

I think it may be useful to expose some stats on the CSS contained within each media query, as this could help illustrate CSS with architectural problems. However, this could be handled in the usage section similar to the other examples you've outlined.

Also, this new approach will take care of /pull/12! 👯

How should the lite option be implemented? Should opts be passed to each module in lib? Or should the modules be split up further?

IMO, the libs should receive the option and handle it accordingly. I think we could also take further steps to modularize this project by breaking a lot of the lib stuff into standalone modules. We should also break the cli into cssstats-cli, allowing us to add more CLI functionality without adding complexity directly to this core module.

what if properties like vendorPrefix or important return an array of occurences

I agree with @rafaelrinaldi here, that would be great!

Usage examples

The usage example section is wonderful, too. I like the idea of not bloating the returned object with data that is less likely to be used. Though, I think we should probably either expose these as options, i.e. { displayNone: true } or another module like cssstats-display-none.

A pluggable API?

I think it might be worthwhile considering some type of pluggable API. That way users could incorporate their own functions and require more esoteric stats examples if they'd like:

var cssstats = require('cssstats')

cssstats(myCss).use(require('cssstats-display-none'), require('cssstats-specificity-sort'), function(stats) {
  // ... Do some crazy stuff with the stats object
})

Not exactly sure how this would work, but somehow each of these "plugins" of additional statistical information could somehow add to the returned stats object.

Thoughts?

@jxnblk
Copy link
Member Author

jxnblk commented Jul 28, 2015

This is super helpful. Thanks!

@rafaelrinaldi @johnotander as far as the vendorPrefix and important arrays go, what should they contain? Both the property and value for each declaration?

My thinking with the shorthand properties and other example is that they might make sense as methods, such as stats.getAllFontSizes(). Any thoughts on that?

I'm down to add more information to the media query object – any ideas on how to parse the contents of a media query block and what to show?

Passing opts into the modules seems like an okay solution. An example of the other idea would be to have both a lib/selectors and a lib/selectors-specificity module. Things like this could also just be methods that are returned with the object.

The idea of a pluggable API is interesting. We could probably rely on the postcss plugin architecture for that – there's probably a sane way to tack on any postcss plugin to cssstats.

@johno
Copy link
Member

johno commented Jul 28, 2015

as far as the vendorPrefix and important arrays go, what should they contain? Both the property and value for each declaration?

I'd say the property and value. As it'd really shed some light on any duplicated declarations with the !important keyword.

My thinking with the shorthand properties and other example is that they might make sense as methods, such as stats.getAllFontSizes(). Any thoughts on that?

👍

I'm down to add more information to the media query object – any ideas on how to parse the contents of a media query block and what to show?

Off the top of my head, I believe all the contained styling is available as .nodes (or .childNodes maybe?) within the atrule object. We could then pass that sub-AST to stats-lite to get some really basic data.

An example of the other idea would be to have both a lib/selectors and a lib/selectors-specificity module. Things like this could also just be methods that are returned with the object.

I think this might be the best approach. Though, we will then have to consider how we convert the returned object to JSON. Perhaps exposing a .toJSON() method? Also, this could potentially be solved if we run with some sort of plugin mechanism, too. Please see below.

We could probably rely on the postcss plugin architecture for that – there's probably a sane way to tack on any postcss plugin to cssstats.

Yeah, I'm not quite sure if this is something that would be worthwhile/useful to end users. Especially if it's not easy to somehow lean on PostCSS to solve this (I will have to do some digging).

Ultimately, the main problem I'm seeing here is that we have different levels/scopes of information that we want to expose in terms of data/stats: Lite, Default, All The Things, Custom Things. It might just be as simple as making options: lite, all, displayNone, etc. However, the drawback I see there is the configuration could be a little unwieldy (but likely not more unwieldy than calling a plugin). Actually, as I type this up, I wonder if this is the best route rather than implementing some type of plugin infrastructure. We can expose a slew of options, with sensible defaults, to configure the statistical information that we return.

@jxnblk
Copy link
Member Author

jxnblk commented Jul 28, 2015

On a side note, I would love to rename this repo cssstats-core if there's no opposition.

@jxnblk
Copy link
Member Author

jxnblk commented Jul 30, 2015

Just published a beta if anyone ones to test it out: npm i cssstats@beta

@johno
Copy link
Member

johno commented Jul 30, 2015

Just did some hand testing with the beta, comparing it to the output with cssstats.com and it looks accurate. Not to mention, the returned object is much more human readable. Awesome work!

:shipit:

@jxnblk
Copy link
Member Author

jxnblk commented Aug 5, 2015

I've added and updated tests and have been trying this out in the new app. Everything feels good to me, and if there's no objections, I'd like to publish an official release today.

README.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

default

@johno
Copy link
Member

johno commented Aug 5, 2015

I'd like to publish an official release today.

👯 👯 👯

jxnblk added a commit that referenced this pull request Aug 5, 2015
@jxnblk jxnblk merged commit 151a60e into master Aug 5, 2015
@jxnblk jxnblk deleted the v2 branch August 5, 2015 17:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants