Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Cleanup how optimize streams work, in preparation for babel stream #524

Merged
merged 8 commits into from
Jan 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 19 additions & 37 deletions src/build/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,75 +13,57 @@
*/

import * as del from 'del';
import * as gulpif from 'gulp-if';
import * as path from 'path';
import * as logging from 'plylog';
import {dest} from 'vinyl-fs';


import mergeStream = require('merge-stream');
import {PolymerProject, addServiceWorker} from 'polymer-build';

import {InlineCSSOptimizeStream, JSOptimizeStream, CSSOptimizeStream, HTMLOptimizeStream} from './optimize-streams';
import {OptimizeOptions, getOptimizeStreams} from './optimize-streams';

import {ProjectConfig} from 'polymer-project-config';
import {PrefetchTransform} from './prefetch';
import {waitFor} from './streams';
import {waitFor, pipeStreams} from './streams';
import {parsePreCacheConfig} from './sw-precache';

const logger = logging.getLogger('cli.build.build');

const buildDirectory = 'build/';

export interface BuildOptions {
export interface BuildOptions extends OptimizeOptions {
swPrecacheConfig?: string;
insertPrefetchLinks?: boolean;
bundle?: boolean;
// TODO(fks) 07-21-2016: Fully complete these with available options
html?: {collapseWhitespace?: boolean; removeComments?: boolean};
css?: {stripWhitespace?: boolean};
js?: {minify?: boolean};
}
};

export async function build(
options: BuildOptions, config: ProjectConfig): Promise<void> {
const optimizeOptions:
OptimizeOptions = {css: options.css, js: options.js, html: options.html};
const polymerProject = new PolymerProject(config);
const swPrecacheConfig = path.resolve(
config.root, options.swPrecacheConfig || 'sw-precache-config.js');

// mix in optimization options from build command
// TODO: let this be set by the user
let optimizeOptions = {
html: Object.assign({removeComments: true}, options.html),
css: Object.assign({stripWhitespace: true}, options.css),
js: Object.assign({minify: true}, options.js),
};

logger.info(`Deleting build/ directory...`);
await del([buildDirectory]);

logger.debug(`Reading source files...`);
let sourcesStream =
polymerProject.sources()
.pipe(polymerProject.splitHtml())
.pipe(gulpif(/\.js$/, new JSOptimizeStream(optimizeOptions.js)))
.pipe(gulpif(/\.css$/, new CSSOptimizeStream(optimizeOptions.css)))
// TODO(fks): Remove this InlineCSSOptimizeStream stream once CSS
// is properly being isolated by splitHtml() & rejoinHtml().
.pipe(gulpif(/\.html$/, new InlineCSSOptimizeStream(optimizeOptions.css)))
.pipe(gulpif(/\.html$/, new HTMLOptimizeStream(optimizeOptions.html)))
.pipe(polymerProject.rejoinHtml());
const sourcesStream = pipeStreams([
polymerProject.sources(),
polymerProject.splitHtml(),
getOptimizeStreams(optimizeOptions),
polymerProject.rejoinHtml()
]);

logger.debug(`Reading dependencies...`);
let depsStream =
polymerProject.dependencies()
.pipe(polymerProject.splitHtml())
.pipe(gulpif(/\.js$/, new JSOptimizeStream(optimizeOptions.js)))
.pipe(gulpif(/\.css$/, new CSSOptimizeStream(optimizeOptions.css)))
// TODO(fks): Remove this InlineCSSOptimizeStream stream once CSS
// is properly being isolated by splitHtml() & rejoinHtml().
.pipe(gulpif(/\.html$/, new InlineCSSOptimizeStream(optimizeOptions.css)))
.pipe(gulpif(/\.html$/, new HTMLOptimizeStream(optimizeOptions.html)))
.pipe(polymerProject.rejoinHtml());
const depsStream = pipeStreams([
polymerProject.dependencies(),
polymerProject.splitHtml(),
getOptimizeStreams(optimizeOptions),
polymerProject.rejoinHtml()
]);

let buildStream: NodeJS.ReadableStream =
mergeStream(sourcesStream, depsStream);
Expand Down
44 changes: 40 additions & 4 deletions src/build/optimize-streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,28 @@
*/

import * as cssSlam from 'css-slam';
import * as gulpif from 'gulp-if';
import {minify as htmlMinify, Options as HTMLMinifierOptions} from 'html-minifier';
import * as logging from 'plylog';
import {Transform} from 'stream';
import {minify as uglify, MinifyOptions as UglifyOptions} from 'uglify-js';


// TODO(fks) 09-22-2016: Latest npm type declaration resolves to a non-module
// entity. Upgrade to proper JS import once compatible .d.ts file is released,
// or consider writing a custom declaration in the `custom_typings/` folder.
import File = require('vinyl');

let logger = logging.getLogger('cli.build.optimize-streams');
const logger = logging.getLogger('cli.build.optimize-streams');

export type FileCB = (error?: any, file?: File) => void;
export type CSSOptimizeOptions = {
stripWhitespace?: boolean;
};
export interface OptimizeOptions {
html?: {minify: boolean};
css?: {minify: boolean};
js?: {minify?: boolean};
};

/**
* GenericOptimizeStream is a generic optimization stream. It can be extended
Expand Down Expand Up @@ -77,11 +82,11 @@ export class JSOptimizeStream extends GenericOptimizeStream {
// uglify is special, in that it returns an object with a code property
// instead of just a code string. We create a compliant optimizer here
// that returns a string instead.
let uglifyOptimizer = (contents: string, options: UglifyOptions) => {
const uglifyOptimizer = (contents: string, options: UglifyOptions) => {
return uglify(contents, options).code;
};
// We automatically add the fromString option because it is required.
let uglifyOptions = Object.assign({fromString: true}, options);
const uglifyOptions = Object.assign({fromString: true}, options);
super('uglify-js', uglifyOptimizer, uglifyOptions);
}
}
Expand Down Expand Up @@ -132,3 +137,34 @@ export class HTMLOptimizeStream extends GenericOptimizeStream {
super('html-minify', htmlMinify, options);
}
}

/**
* Returns an array of optimization streams to use in your build, based on the
* OptimizeOptions given.
*/
export function getOptimizeStreams(options?: OptimizeOptions):
NodeJS.ReadWriteStream[] {
options = options || {};
const streams = [];

// add optimizers
if (options.html && options.html.minify) {
streams.push(gulpif(
/\.html$/,
new HTMLOptimizeStream(
{collapseWhitespace: true, removeComments: true})));
}
if (options.css && options.css.minify) {
streams.push(
gulpif(/\.css$/, new CSSOptimizeStream({stripWhitespace: true})));
// TODO(fks): Remove this InlineCSSOptimizeStream stream once CSS
// is properly being isolated by splitHtml() & rejoinHtml().
streams.push(gulpif(
/\.html$/, new InlineCSSOptimizeStream({stripWhitespace: true})));
}
if (options.js && options.js.minify) {
streams.push(gulpif(/\.js$/, new JSOptimizeStream({fromString: true})));
}

return streams;
};
16 changes: 16 additions & 0 deletions src/build/streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,19 @@ export function waitForAll(streams: NodeJS.ReadableStream[]):
Promise<NodeJS.ReadableStream[]> {
return Promise.all<NodeJS.ReadableStream>(streams.map((s) => waitFor(s)));
}

type PipeStream = (NodeJS.ReadableStream|NodeJS.WritableStream|
NodeJS.ReadableStream[]|NodeJS.WritableStream[]);

/**
* pipeStreams() takes in a collection streams and pipes them together,
* returning the last stream in the pipeline. Each element in the `streams`
* array must be either a stream, or an array of streams (see PipeStream).
* pipeStreams() will then flatten this array before piping them all together.
*/
export function pipeStreams(streams: PipeStream[]): NodeJS.ReadableStream {
return Array.prototype.concat.apply([], streams)
.reduce((a: NodeJS.ReadableStream, b: NodeJS.ReadWriteStream) => {
return a.pipe(b);
});
}
36 changes: 25 additions & 11 deletions src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ export class BuildCommand implements Command {
description = 'Builds an application-style project';

args = [
{
name: 'js-minify',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not so sure about this change. The reason for namespacing flags like html.collapseWhitespace was to at some point be able to easily map flags to build options. Continuing that pattern would lead to js.minify, css.minify and html.minify.

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 really dislike the . in the command line flag, a dash feels much more natural and doesn't break the expected CLI dash-case naming convention. We can still map easily regardless of . or -.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's that uncommon to use . when flags map to JSON-like structures. Mapping with - is harder: does foo-bar-baz map to foo.bar.baz, foo.bar-baz, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the user will never know about or use that JSON-like structure, right? If we were maintaining a large set of rules or mapping to an interface that the user was familiar with I would agree, but I don't see the point in breaking our expected naming system by mapping to an internal interface that could change at any time.

Copy link
Contributor

Choose a reason for hiding this comment

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

The user probably will, when we explain that the flags map to the options in polymer.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I hadn't assumed we'd use the same options for build() in polymer.json.

Can we table this decision then until we start on the polymer.json options? I think that will be a more productive conversation when those options actually exist vs. planning for something we haven't really thought through yet (at least I haven't yet).

Copy link
Contributor

Choose a reason for hiding this comment

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

We can, but the pattern had been set up with that goal in mind. If we defer the discussion can we stick with the current pattern?

Copy link
Contributor Author

@FredKSchott FredKSchott Dec 21, 2016

Choose a reason for hiding this comment

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

sure thing, rolling back
discussed and planned out in person, our flags will use this format (https://docs.google.com/document/d/1mD1cVhxdxY-bUsGBX44anC1875Afx7UHue4uRCNVc4o). Updating the BuildOptions to match the planned polymer.json options now.

type: Boolean,
description: 'minify inlined and external JavaScript.'
},
{
name: 'css-minify',
type: Boolean,
description: 'minify inlined and external CSS.'
},
{
name: 'html-minify',
type: Boolean,
description: 'minify HTML by removing comments and whitespace.'
},
{
name: 'bundle',
defaultValue: false,
Expand All @@ -52,11 +67,6 @@ export class BuildCommand implements Command {
'`<link rel="import">` tags into fragments and shell for all ' +
'dependencies.'
},
{
name: 'html.collapseWhitespace',
type: Boolean,
description: 'Collapse whitespace in HTML files'
}
];

run(options: CommandOptions, config: ProjectConfig): Promise<any> {
Expand All @@ -67,13 +77,17 @@ export class BuildCommand implements Command {
swPrecacheConfig: options['sw-precache-config'],
insertPrefetchLinks: options['insert-prefetch-links'],
bundle: options['bundle'],
html: {},
css: {},
js: {},
html: {
minify: !!options['html-minify'],
},
css: {
minify: !!options['css-minify'],
},
js: {
minify: !!options['js-minify'],
},
};
if (options['html.collapseWhitespace']) {
buildOptions.html!.collapseWhitespace = true;
}

logger.debug('building with options', buildOptions);

if (options['env'] && options['env'].build) {
Expand Down