Skip to content

Commit

Permalink
Fix: Ensure all css is written and deduped correctly in BTR (#106)
Browse files Browse the repository at this point in the history
* Remove primitive logic for deduping css, instead use postcss and cssnano

* code style

* fix version
  • Loading branch information
agubler authored Dec 21, 2018
1 parent 3db46bb commit 85ee025
Show file tree
Hide file tree
Showing 11 changed files with 1,277 additions and 242 deletions.
1,395 changes: 1,236 additions & 159 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"chalk": "2.3.0",
"commander": "2.13.0",
"copy-webpack-plugin": "4.6.0",
"cssnano": "4.1.7",
"express": "4.16.2",
"filesize": "3.5.11",
"filter-css": "0.1.2",
Expand All @@ -64,6 +65,7 @@
"lodash": "4.17.4",
"mkdirp": "0.5.1",
"opener": "1.4.3",
"postcss": "7.0.7",
"puppeteer": "1.10.0",
"recast": "0.12.7",
"source-map": "0.6.1",
Expand All @@ -76,10 +78,11 @@
"devDependencies": {
"@dojo/scripts": "~3.1.0",
"@types/acorn": "^4.0.3",
"@types/cssnano": "4.0.0",
"@types/estree": "0.0.37",
"@types/express": "^4.16.0",
"@types/fs-extra": "^5.0.4",
"@types/get-port": "^4.0.0",
"@types/get-port": "4.0.0",
"@types/glob": "^5.0.35",
"@types/gzip-size": "^4.1.0",
"@types/loader-utils": "^1.1.3",
Expand Down
38 changes: 16 additions & 22 deletions src/build-time-render/BuildTimeRender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import {
getForSelector,
setHasFlags
} from './helpers';
import * as cssnano from 'cssnano';
const filterCss = require('filter-css');
const puppeteer = require('puppeteer');
const webpack = require('webpack');
const SourceNode = require('source-map').SourceNode;
const SourceMapConsumer = require('source-map').SourceMapConsumer;
const postcss = require('postcss');

export interface RenderResult {
path?: string | BuildTimePath;
Expand Down Expand Up @@ -44,7 +46,6 @@ export default class BuildTimeRender {
private _cssFiles: string[] = [];
private _entries: string[];
private _head: string;
private _inlinedCssClassNames: string[] = [];
private _manifest: any;
private _manifestContent: any = {};
private _buildBridgeResult: any = {};
Expand All @@ -68,7 +69,7 @@ export default class BuildTimeRender {
this._useHistory = useHistory !== undefined ? useHistory : paths.length > 0 && !/^#.*/.test(initialPath);
}

private _writeIndexHtml({ html, script, path = '', styles }: RenderResult) {
private async _writeIndexHtml({ html, script, path = '', styles }: RenderResult) {
path = typeof path === 'object' ? path.path : path;
const prefix = getPrefix(path);
if (this._head) {
Expand All @@ -85,6 +86,7 @@ export default class BuildTimeRender {
return css;
}, '');

styles = await this._processCss(styles);
html = html.replace(`</head>`, `<style>${styles}</style></head>`);
html = html.replace(/^(\s*)(\r\n?|\n)/gm, '').trim();
html = html.replace(this._createScripts(path), `${script}${css}${this._createScripts(path)}`);
Expand All @@ -110,27 +112,21 @@ export default class BuildTimeRender {
.slice(0, 2)
.join('.');
const firstChar = value.substr(0, 1);
if (!this._useHistory && this._inlinedCssClassNames.indexOf(value) !== -1) {
return true;
}
if (classes.indexOf(value) !== -1 || ['.', '#'].indexOf(firstChar) === -1) {
this._inlinedCssClassNames.push(value);
return false;
}
return true;

return classes.indexOf(value) === -1 && ['.', '#'].indexOf(firstChar) !== -1;
}
});

filteredCss = filteredCss
.replace(/\/\*.*\*\//g, '')
.replace(/^(\s*)(\r\n?|\n)/gm, '')
.trim();
result = `${result}${filteredCss}
`;
return result;
return `${result}${filteredCss}`;
}, '');
}

private async _processCss(css: string) {
const cssnanoConfig = cssnano({ preset: ['default', { calc: false, normalizeUrl: false }] });
const processedCss = await postcss([cssnanoConfig]).process(css, { from: undefined });
return processedCss.css;
}

private async _getRenderResult(
page: any,
path: BuildTimePath | string | undefined = undefined,
Expand Down Expand Up @@ -253,7 +249,7 @@ export default class BuildTimeRender {

if (this._paths.length === 0) {
const result = await this._getRenderResult(page, undefined);
this._writeIndexHtml(result);
await this._writeIndexHtml(result);
} else {
let renderResults: RenderResult[] = [];
renderResults.push(await this._getRenderResult(page, undefined, this._useHistory));
Expand All @@ -266,9 +262,7 @@ export default class BuildTimeRender {
}

if (this._useHistory) {
renderResults.forEach((result) => {
this._writeIndexHtml(result);
});
await renderResults.map((result) => this._writeIndexHtml(result));
} else {
const combined = renderResults.reduce(
(combined, result) => {
Expand All @@ -287,7 +281,7 @@ export default class BuildTimeRender {
);
let html = readFileSync(join(this._output, 'index.html'), 'utf-8');
const script = generateRouteInjectionScript(combined.html, combined.paths, this._root);
this._writeIndexHtml({ styles: combined.styles, html, script });
await this._writeIndexHtml({ styles: combined.styles, html, script });
}
}
if (Object.keys(this._buildBridgeResult).length) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
<html><head>
<style>span {
width: 100%;
}
</style></head>
<style>span{width:100%}</style></head>
<body>
<div id="app"><div>hello world a</div></div>
<script type="text/javascript" src="runtime.js"></script><script type="text/javascript" src="main.js"></script>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
<html><head>
<style>.hello {
background: red;
}
span {
width: 100%;
}
</style></head>
<style>.hello{background:red}.link{line-height:20px;color:#00b0e8}span{width:100%;height:100%}</style></head>
<body>
<div id="app"><div class="hello">Hello</div></div>
<div id="app"><div class="hello link">Hello</div></div>
<link rel="stylesheet" href="main.css" media="none" onload="if(media!='all')media='all'" /><script type="text/javascript" src="runtime.js"></script><script type="text/javascript" src="main.js"></script>
</body></html>
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
<html>
<head>
<style>
.hello {
background: red;
}
span {
width: 100%;
}
.other {
color: pink;
}
</style></head>
<style>.hello{background:red}.link{line-height:20px;color:#00b0e8}.other{color:pink}span{width:100%;height:100%}</style></head>
<body>
<div id="app"></div>
<script>
(function () {
var paths = ["",{"path":"#my-path"}];
var html = ["<div id=\"app\"><div class=\"hello\">Hello</div></div>","<div id=\"app\"><div class=\"other\">Root</div></div>"];
var html = ["<div id=\"app\"><div class=\"hello link\">Hello</div></div>","<div id=\"app\"><div class=\"other\">Root</div></div>"];
var element = document.getElementById('app');
var target;
paths.some(function (path, i) {
Expand Down
12 changes: 12 additions & 0 deletions tests/support/fixtures/build-time-render/hash/main.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tests/support/fixtures/build-time-render/hash/main.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(function main() {
const app = document.getElementById('app');
let div = document.createElement('div');
div.classList.add('hello');
div.classList.add('hello', 'link');
div.innerHTML = 'Hello';
app.appendChild(div);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
<html><head>
<link rel="manifest" href="../manifest.json" />
<style>.hello {
background: red;
}
span {
width: 100%;
}
.another {
background: red;
}
</style></head>
<style>.hello{background:red}span{width:100%}.another{background:red}</style></head>
<body>
<div id="app"><div class="hello another">{"staticFeatures":{"build-time-render":true}}</div></div>
<script>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
<html><head>
<link rel="manifest" href="../../manifest.json" />
<style>.other.another {
color: pink;
background: url("../../relative.png");
background: url("/root.png");
background: url("../.././relative.png");
background: url("../../../relative.png");
background: url("https://example.com");
background: url("http://example.com");
background: url(https://example.com);
background: url(http://example.com);
}
span {
width: 100%;
}
</style></head>
<style>.other.another{color:pink;background:url("../../relative.png");background:url("/root.png");background:url("../.././relative.png");background:url("../../../relative.png");background:url("https://example.com");background:url("http://example.com");background:url(https://example.com);background:url(http://example.com)}span{width:100%}</style></head>
<body>
<div id="app"><div class="other">Other</div></div>
<script>
Expand Down
11 changes: 1 addition & 10 deletions tests/support/fixtures/build-time-render/state/other/index.html
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
<html><head>
<link rel="manifest" href="../manifest.json" />
<style>.hello {
background: red;
}
span {
width: 100%;
}
.another {
background: red;
}
</style></head>
<style>.hello{background:red}span{width:100%}.another{background:red}</style></head>
<body>
<div id="app"><div class="hello another">{"staticFeatures":{"build-time-render":true}}<img src="https://example.com"><img src="http://example.com"><img src="/image.svg"><img src=".././relative-image.svg"><img src="../../other-relative-image.svg"></div></div>
<script>
Expand Down

0 comments on commit 85ee025

Please sign in to comment.