Skip to content

Conversation

@shawnbot
Copy link
Contributor

@shawnbot shawnbot commented Feb 12, 2019

This refactors our CSS build script and reworks how we talk about "packages" internally so that (I think) it's clearer.

TL;DR: "packages" are now known as "bundles" — at least internally, for now. The word "package" tends to refer specifically to npm packages (or "modules"), and the word "bundle" aligns more closely with how we talk about JS and CSS builds that include specific things.

Here's what's in the tin:

  1. It uses postcss and (basically) the same config as GitHub's asset pipeline.
  2. It introduces a standalone .browserslistrc which exposes our browser support matrix. (This file is picked up automatically by autoprefixer.)
  3. Our postcss config is published as a separate file can be referenced (e.g. from the postcss CLI) via @primer/css/postcss.config.
  4. It creates one instance of the postcss processor and reuses it (rather than shelling out to the postcss CLI for each file), which cuts the build time from over 12 seconds to ~2! ✨
  5. I've reworked how the dist/ directory is laid out:
    • dist/{name}.css is still the CSS build for each bundle, e.g. dist/primer.css or dist/marketing.css.
    • dist/{name}.css.map is the sourcemap, and references all of the source SCSS files relatively.
    • dist/{name}.js is the JS export for each bundle, which (still) only has a cssstats key. In other words, you can require('@primer/css/dist/utilities') to get the utility stats.
    • dist/stats/{name}.json is the cssstats output as raw JSON, and is require()-d by the JS.
  6. There's an additional dist/meta.json that currently only has one key: bundles is an object with keys like core, product, marketing, and utilities that each contain information about the bundle (all paths are relative to the root, not dist/):
    • its source path
    • its import path in Sass apps (@primer/css/blankslate/index.scss)
    • its legacy import path (primer-blankslate/index.scss)
    • its CSS bundle and sourcemap paths
    • its JS entrypoint for stats
    • its raw JSON stats path
    • the names of other bundles that it imports (its "dependencies")
  7. For backwards compatibility, the prepublish script copies the relevant files to:
    • build/build.css
    • build/data.json
    • build/index.js

@shawnbot shawnbot requested a review from jonrohan February 12, 2019 05:45
@shawnbot shawnbot changed the title [WIP] Refactor build scripts w/postcss Refactor build scripts w/postcss Feb 12, 2019
@shawnbot shawnbot mentioned this pull request Feb 12, 2019
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Changes look mostly good. Left a question and an opinion. Ship when you're ready.

# TODO: update this to pull from @primer/css
old_path="primer/build/data.json"
log "Pulling the old $old_path ..."
curl -sL "https://unpkg.com/$old_path" > before.json
Copy link
Member

Choose a reason for hiding this comment

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

Not to be down on unpkg, but some future version I'd love to see stuff pulled from github tags (or releases)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hear you, but we need the published files in this case, which are not part of the GitHub release. (Though we might consider that a bug, because maybe they should be!) I'm mostly being lazy here because unpkg is just the easiest way to get at a single file; if we're not down with unpkg, we should consider grabbing the tarball from npm directly and pulling out the file we want.

@shawnbot shawnbot merged commit 5b8e729 into reorg Feb 12, 2019
@shawnbot shawnbot deleted the reorg-build branch February 12, 2019 19:38
shawnbot added a commit that referenced this pull request Feb 12, 2019
Refactor build scripts w/postcss
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.

4 participants