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

Refactor build scripts w/postcss #672

Merged
merged 25 commits into from
Feb 12, 2019
Merged

Refactor build scripts w/postcss #672

merged 25 commits into from
Feb 12, 2019

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.

package.json Show resolved Hide resolved
# 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.

3 participants