Skip to content

Commit

Permalink
fix: rollup minify selectors should retain spaces (#3524)
Browse files Browse the repository at this point in the history
# Description
Reduce complexity for CSS tagged template compression. Also spotted some invalid commas in the `progress` stylesheets.

## Motivation & context
The CSS tagged template minifier was combining `:host() .child` selectors, causing styles to not get rendered properly.

## Issue type checklist

<!--- What type of change are you submitting? Put an x in the box that applies: -->

- [ ] **Chore**: A change that does not impact distributed packages.
- [x] **Bug fix**: A change that fixes an issue, link to the issue above.
- [ ] **New feature**: A change that adds functionality.

**Is this a breaking change?**
- [ ] This change causes current functionality to break.

<!--- If yes, describe the impact. -->

**Adding or modifying component(s) in `@microsoft/fast-components` checklist**

<!-- Do your changes add or modify components in the @microsoft/fast-components package? Put an x in the box that applies: -->

- [ ] I have added a new component
- [ ] I have modified an existing component
- [ ] I have updated the [definition file](https://github.com/Microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#definition)
- [ ] I have updated the [configuration file](https://github.com/Microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#configuration)

## Process & policy checklist

<!--- Review the list and check the boxes that apply. -->

- [ ] I have added tests for my changes.
- [x] I have tested my changes.
- [ ] I have updated the project documentation to reflect my changes.
- [x] I have read the [CONTRIBUTING](https://github.com/Microsoft/fast/blob/master/CONTRIBUTING.md) documentation and followed the [standards](https://www.fast.design/docs/en/contributing/standards) for this project.

<!---
Formatting guidelines:

Accepted peer review title format:
<type>: <description>

Example titles:
    chore: add unit tests for all components
    feat: add a border radius to button
    fix: update design system to use 3px border radius

    <type> is required to be one of the following:

        - chore: A change that does not impact distributed packages.
        - fix: A change which fixes an issue.
        - feat: A that adds functionality.

    <description> is required for the CHANGELOG and speaks to what the user gets from this PR:

        - Be concise.
        - Use all lowercase characters. 
        - Use imperative, present tense (e.g. `add` not `adds`.)
        - Do not end your description with a period.
        - Avoid redundant words.

For additional information regarding working on FAST, check out our documentation site:
https://www.fast.design/docs/en/contributing/working
-->
  • Loading branch information
radium-v authored Jul 20, 2020
1 parent ea84cbd commit cbdfc45
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const spaceBetweenTags = /(>)\s+(<)/g;
const spaceBetweenAttrs = /(["'\w])(?!\s*>)\s+/g;
const openEnded = /(?:[^="'\w])?(["'\w])\s*$/g;

export default function transformHTMLFragment(data) {
export function transformHTMLFragment(data) {
// if the chunk is only space, collapse and return it
if (data.match(onlySpace)) {
return data.replace(onlySpace, " ");
Expand All @@ -25,3 +25,25 @@ export default function transformHTMLFragment(data) {

return data.trim();
}

const newlines = /\n/g;
const separators = /\s*([\{\};])\s*/g;
const lastProp = /;\s*(\})/g;
const extraSpaces = /\s\s+/g;
const endingSpaces = / ?\s+$/g;

export function transformCSSFragment(data) {
// newlines
data = data.replace(newlines, "");

// Remove extra space, but not too much
data = data.replace(separators, "$1");

// Remove semicolons followed by property list end
data = data.replace(lastProp, "$1");

// space might be between property values or between selectors
data = data.replace(endingSpaces, " ");

return data.replace(extraSpaces, " ");
}
1 change: 0 additions & 1 deletion packages/web-components/fast-components-msft/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
"rollup": "^2.7.6",
"rollup-plugin-commonjs": "^10.1.0",
"rollup-plugin-filesize": "^8.0.2",
"rollup-plugin-minify-tagged-css-template": "^0.0.2",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-terser": "^5.3.0",
"rollup-plugin-transform-tagged-template": "^0.0.3",
Expand Down
19 changes: 14 additions & 5 deletions packages/web-components/fast-components-msft/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import commonJS from "rollup-plugin-commonjs";
import filesize from "rollup-plugin-filesize";
import minifyTaggedCSSTemplate from "rollup-plugin-minify-tagged-css-template";
import transformTaggedTemplate from "rollup-plugin-transform-tagged-template";
import resolve from "rollup-plugin-node-resolve";
import { terser } from "rollup-plugin-terser";
import transformTaggedTemplate from "rollup-plugin-transform-tagged-template";
import typescript from "rollup-plugin-typescript2";
import transformHTMLFragment from "../../../build/transform-html-fragment";
import {
transformCSSFragment,
transformHTMLFragment,
} from "../../../build/transform-fragments";

const parserOptions = { sourceType: "module" };
const parserOptions = {
sourceType: "module",
};

export default [
{
Expand All @@ -33,14 +37,19 @@ export default [
},
},
}),
minifyTaggedCSSTemplate({ parserOptions }),
transformTaggedTemplate({
tagsToProcess: ["css"],
transformer: transformCSSFragment,
parserOptions,
}),
transformTaggedTemplate({
tagsToProcess: ["html"],
transformer: transformHTMLFragment,
parserOptions,
}),
filesize({
showMinifiedSize: false,
showBrotliSize: true,
}),
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const ProgressStyles = css`
height: 100%;
background-color: ${accentForegroundRestBehavior.var};
border-radius: calc(var(--design-unit) * 1px);
animation-timing-function: cubic-bezier(0.4, 0.0, 0.6, 1.0);
animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1);
width: 40%;
animation: indeterminate-1 2s infinite;
}
Expand All @@ -59,12 +59,13 @@ export const ProgressStyles = css`
height: 100%;
background-color: ${accentForegroundRestBehavior.var};
border-radius: calc(var(--design-unit) * 1px);
animation-timing-function: cubic-bezier(0.4, 0.0, 0.6, 1.0);
animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1);
width: 60%;
animation: indeterminate-2 2s infinite;
}
:host(.paused) .indeterminate-indicator-1, :host(.paused) .indeterminate-indicator-2 {
:host(.paused) .indeterminate-indicator-1,
:host(.paused) .indeterminate-indicator-2 {
animation-play-state: paused;
background-color: ${neutralFillRestBehavior.var};
}
Expand All @@ -88,7 +89,7 @@ export const ProgressStyles = css`
100% {
opacity: 0;
transform: translateX(300%);
},
}
}
@keyframes indeterminate-2 {
Expand All @@ -106,8 +107,8 @@ export const ProgressStyles = css`
100% {
transform: translateX(166.66%);
opacity: 1;
},
},
}
}
`.withBehaviors(
accentForegroundRestBehavior,
neutralFillRestBehavior,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ import {
} from "../styles";

export const SliderStyles = css`
:host([hidden]) {
display: none;
}
${display("inline-grid")} :host {
--thumb-size: calc(${heightNumber} * 0.5);
--thumb-translate: calc(var(--thumb-size) * 0.5);
Expand Down Expand Up @@ -57,7 +53,7 @@ export const SliderStyles = css`
position: absolute;
height: calc(var(--thumb-size) * 1px);
width: calc(var(--thumb-size) * 1px);
transition: "all 0.2s ease";
transition: all 0.2s ease;
}
.thumb-cursor {
border: none;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ export const BaseButtonStyles = css`
.start,
.end,
::slotted(svg) {
/* Glyph size and margin-left is temporary -
replace when adaptive typography is figured out */
width: 16px;
${
/* Glyph size and margin-left is temporary -
replace when adaptive typography is figured out */ ""
} width: 16px;
height: 16px;
}
Expand Down
1 change: 0 additions & 1 deletion packages/web-components/fast-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
"rollup": "^2.7.6",
"rollup-plugin-commonjs": "^10.1.0",
"rollup-plugin-filesize": "^8.0.2",
"rollup-plugin-minify-tagged-css-template": "^0.0.2",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-terser": "^5.3.0",
"rollup-plugin-transform-tagged-template": "^0.0.3",
Expand Down
21 changes: 16 additions & 5 deletions packages/web-components/fast-components/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import commonJS from "rollup-plugin-commonjs";
import filesize from "rollup-plugin-filesize";
import minifyTaggedCSSTemplate from "rollup-plugin-minify-tagged-css-template";
import resolve from "rollup-plugin-node-resolve";
import { terser } from "rollup-plugin-terser";
import transformTaggedTemplate from "rollup-plugin-transform-tagged-template";
import typescript from "rollup-plugin-typescript2";
import transformHTMLFragment from "../../../build/transform-html-fragment";
import {
transformCSSFragment,
transformHTMLFragment,
} from "../../../build/transform-fragments";

const parserOptions = { sourceType: "module" };
const parserOptions = {
sourceType: "module",
};

export default [
{
Expand All @@ -33,13 +37,20 @@ export default [
},
},
}),
minifyTaggedCSSTemplate({ parserOptions }),
transformTaggedTemplate({
tagsToProcess: ["css"],
transformer: transformCSSFragment,
parserOptions,
}),
transformTaggedTemplate({
tagsToProcess: ["html"],
transformer: transformHTMLFragment,
parserOptions,
}),
filesize({ showMinifiedSize: false }),
filesize({
showMinifiedSize: false,
showBrotliSize: true,
}),
],
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const ProgressStyles = css`
height: 100%;
background-color: ${accentForegroundRestBehavior.var};
border-radius: calc(var(--design-unit) * 1px);
animation-timing-function: cubic-bezier(0.4, 0.0, 0.6, 1.0);
animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1);
width: 40%;
animation: indeterminate-1 2s infinite;
}
Expand All @@ -59,12 +59,13 @@ export const ProgressStyles = css`
height: 100%;
background-color: ${accentForegroundRestBehavior.var};
border-radius: calc(var(--design-unit) * 1px);
animation-timing-function: cubic-bezier(0.4, 0.0, 0.6, 1.0);
animation-timing-function: cubic-bezier(0.4, 0, 0.6, 1);
width: 60%;
animation: indeterminate-2 2s infinite;
}
:host(.paused) .indeterminate-indicator-1, :host(.paused) .indeterminate-indicator-2 {
:host(.paused) .indeterminate-indicator-1,
:host(.paused) .indeterminate-indicator-2 {
animation-play-state: paused;
background-color: ${neutralFillRestBehavior.var};
}
Expand All @@ -88,7 +89,7 @@ export const ProgressStyles = css`
100% {
opacity: 0;
transform: translateX(300%);
},
}
}
@keyframes indeterminate-2 {
Expand All @@ -106,8 +107,8 @@ export const ProgressStyles = css`
100% {
transform: translateX(166.66%);
opacity: 1;
},
},
}
}
`.withBehaviors(
accentForegroundRestBehavior,
neutralFillRestBehavior,
Expand Down
2 changes: 1 addition & 1 deletion packages/web-components/fast-element/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@
"prettier": "2.0.2",
"rollup": "^2.7.6",
"rollup-plugin-filesize": "^8.0.2",
"rollup-plugin-minify-tagged-css-template": "^0.0.2",
"rollup-plugin-terser": "^5.3.0",
"rollup-plugin-transform-tagged-template": "^0.0.3",
"rollup-plugin-typescript2": "^0.27.0",
"source-map": "^0.7.3",
"source-map-loader": "^0.2.4",
Expand Down
28 changes: 23 additions & 5 deletions packages/web-components/fast-element/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import commonJS from "rollup-plugin-commonjs";
import filesize from "rollup-plugin-filesize";
import minifyTaggedCSSTemplate from "rollup-plugin-minify-tagged-css-template";
import resolve from "rollup-plugin-node-resolve";
import { terser } from "rollup-plugin-terser";
import transformTaggedTemplate from "rollup-plugin-transform-tagged-template";
import typescript from "rollup-plugin-typescript2";
import {
transformCSSFragment,
transformHTMLFragment,
} from "../../../build/transform-fragments";

const parserOptions = {
sourceType: "module",
};

export default [
{
Expand All @@ -18,20 +28,28 @@ export default [
},
],
plugins: [
resolve(),
commonJS(),
typescript({
tsconfigOverride: {
compilerOptions: {
declaration: false,
},
},
}),
minifyTaggedCSSTemplate({
parserOptions: {
sourceType: "module",
},
transformTaggedTemplate({
tagsToProcess: ["css"],
transformer: transformCSSFragment,
parserOptions,
}),
transformTaggedTemplate({
tagsToProcess: ["html"],
transformer: transformHTMLFragment,
parserOptions,
}),
filesize({
showMinifiedSize: false,
showBrotliSize: true,
}),
],
},
Expand Down
1 change: 0 additions & 1 deletion packages/web-components/fast-foundation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
"rollup": "^2.7.6",
"rollup-plugin-commonjs": "^10.1.0",
"rollup-plugin-filesize": "^8.0.2",
"rollup-plugin-minify-tagged-css-template": "^0.0.2",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-terser": "^5.3.0",
"rollup-plugin-transform-tagged-template": "^0.0.3",
Expand Down
17 changes: 13 additions & 4 deletions packages/web-components/fast-foundation/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import commonJS from "rollup-plugin-commonjs";
import filesize from "rollup-plugin-filesize";
import minifyTaggedCSSTemplate from "rollup-plugin-minify-tagged-css-template";
import resolve from "rollup-plugin-node-resolve";
import { terser } from "rollup-plugin-terser";
import transformTaggedTemplate from "rollup-plugin-transform-tagged-template";
import typescript from "rollup-plugin-typescript2";
import transformHTMLFragment from "../../../build/transform-html-fragment";
import {
transformCSSFragment,
transformHTMLFragment,
} from "../../../build/transform-fragments";

const parserOptions = { sourceType: "module" };
const parserOptions = {
sourceType: "module",
};

export default [
{
Expand All @@ -33,14 +37,19 @@ export default [
},
},
}),
minifyTaggedCSSTemplate({ parserOptions }),
transformTaggedTemplate({
tagsToProcess: ["css"],
transformer: transformCSSFragment,
parserOptions,
}),
transformTaggedTemplate({
tagsToProcess: ["html"],
transformer: transformHTMLFragment,
parserOptions,
}),
filesize({
showMinifiedSize: false,
showBrotliSize: true,
}),
],
},
Expand Down
7 changes: 0 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -20496,13 +20496,6 @@ rollup-plugin-filesize@^8.0.2:
pacote "^11.1.6"
terser "^4.6.12"

rollup-plugin-minify-tagged-css-template@^0.0.2:
version "0.0.2"
resolved "https://registry.yarnpkg.com/rollup-plugin-minify-tagged-css-template/-/rollup-plugin-minify-tagged-css-template-0.0.2.tgz#72d0890c1d97c0022ad4b1e10f41526e1cecdff5"
integrity sha512-0RgsTGTV83AWcDHoRsdAOydtOXkZOoF5XqcixvcxzIgjRmTD19htplR241C7Q1fBnK7aVtJXAWqkRAsdXpCxcQ==
dependencies:
rollup-plugin-transform-tagged-template "^0.0.3"

rollup-plugin-node-resolve@^5.2.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/rollup-plugin-node-resolve/-/rollup-plugin-node-resolve-5.2.0.tgz#730f93d10ed202473b1fb54a5997a7db8c6d8523"
Expand Down

0 comments on commit cbdfc45

Please sign in to comment.