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

add dist:css step to prod-ify CSS file #2241

Merged
merged 11 commits into from
Mar 15, 2018
Merged

add dist:css step to prod-ify CSS file #2241

merged 11 commits into from
Mar 15, 2018

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Mar 14, 2018

Fixes #612, fixes #2122

Changes proposed in this pull request:

  • a few fixes in node-build-scripts:
    • add output option to sass-compile
    • add more main fields to assert-package-layout
    • add auto-fix string for file-header rule in tslint-config
  • sass-dist script runs postcss with autoprefixer and discard-comments plugins
  • add important comment at the top of each scss entry file (preserved in dist output)
  • add .browserslistrc file instead of individual autoprefixer config

Reviewers should focus on:

blueprint-datetime.css   185K => 18K
blueprint-icons.css      856B => 988B *
blueprint-labs.css        47B => 162B *
blueprint-select.css      54K => 4.6K
blueprint-timezone.css    57K => 323B
blueprint.css            1.4M => 432K **
docs-theme.css           233K => 29K
table.css                313K => 42K

* - file size increase due to inline sourcemap
** - 70% filesize reduction!!!

@blueprint-bot
Copy link

autoprefixer 7 for hoisting

Preview: documentation | landing | table

@reiv
Copy link
Contributor

reiv commented Mar 14, 2018

This would fix #2122 as well. ✨

(Edit: for those that don't already use autoprefixer, I mean.)

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

nice! just small comments. I think the new script should be called css-dist

.browserslistrc Outdated
@@ -0,0 +1,4 @@
# Browsers that we support

defaults
Copy link
Contributor

Choose a reason for hiding this comment

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

can we be more explicit? I don't really want to have to look at the documentation for the default. also more comments in this file would be really helpful, to explain its context

@@ -0,0 +1,5 @@
#!/usr/bin/env bash

# Usage: sass-compile [outdir=lib/css/]
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this more like css-dist? it has nothing to do with sass.

@giladgray
Copy link
Contributor Author

yarn cache issue with renamed bin script causing build fails

# ensure these are supported:
Firefox ESR
IE 11
# because no one likes a dead browser:
Copy link
Contributor

Choose a reason for hiding this comment

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

lol

@adidahiya
Copy link
Contributor

that's annoying... let's merge into develop and fix things there if necessary

@adidahiya adidahiya merged commit 0a6f4de into develop Mar 15, 2018
@adidahiya adidahiya deleted the gg/postcss-build branch March 15, 2018 20:09
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.

user-select needs vendor prefixing Add minified bundles to distribution
4 participants