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.

Sign up for GitHub

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

Jump to bottom

feat(build): allow output hashing to be configured #3885

Merged
merged 4 commits into from Jan 10, 2017
Merged

feat(build): allow output hashing to be configured #3885

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
47820b8
feat: allow output hashing to be configured
Jan 6, 2017
e8bb961
cleanup and address comments
Jan 7, 2017
0690808
add tests
Jan 10, 2017
90d3e7e
add docs
Jan 10, 2017
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
9 changes: 9 additions & 0 deletions packages/angular-cli/commands/build.run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ export default function buildRun(commandOptions: BuildOptions) {
}
}

if (!commandOptions.outputHashing) {
if (commandOptions.target === 'development') {
commandOptions.outputHashing = 'none';
}
if (commandOptions.target === 'production') {
commandOptions.outputHashing = 'all';
}
}

const project = this.project;

// Check angular version.
Expand Down
9 changes: 8 additions & 1 deletion packages/angular-cli/commands/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface BuildOptions {
i18nFormat?: string;
locale?: string;
deployUrl?: string;
outputHashing?: string;
}

const BuildCommand = Command.extend({
Expand Down Expand Up @@ -45,7 +46,13 @@ const BuildCommand = Command.extend({
{ name: 'i18n-file', type: String, default: null },
{ name: 'i18n-format', type: String, default: null },
{ name: 'locale', type: String, default: null },
{ name: 'deploy-url', type: String, default: null, aliases: ['d'] }
{ name: 'deploy-url', type: String, default: null, aliases: ['d'] },
{
name: 'output-hashing',
type: String,
values: ['none', 'all', 'media', 'bundles'],
description: 'Sets the hashing mode for output filenames'
}
],

run: function (commandOptions: BuildOptions) {
Expand Down
27 changes: 21 additions & 6 deletions packages/angular-cli/models/webpack-build-common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import { GlobCopyWebpackPlugin } from '../plugins/glob-copy-webpack-plugin';
import { SuppressEntryChunksWebpackPlugin } from '../plugins/suppress-entry-chunks-webpack-plugin';
import { packageChunkSort } from '../utilities/package-chunk-sort';
import { BaseHrefWebpackPlugin } from '@angular-cli/base-href-webpack';
import { extraEntryParser, makeCssLoaders } from './webpack-build-utils';
import { extraEntryParser, makeCssLoaders, getOutputHashFormat } from './webpack-build-utils';

const autoprefixer = require('autoprefixer');
const ProgressPlugin = require('webpack/lib/ProgressPlugin');
const HtmlWebpackPlugin = require('html-webpack-plugin');
const ExtractTextPlugin = require('extract-text-webpack-plugin');
const SilentError = require('silent-error');

/**
Expand All @@ -31,7 +32,8 @@ export function getWebpackCommonConfig(
sourcemap: boolean,
vendorChunk: boolean,
verbose: boolean,
progress: boolean
progress: boolean,
outputHashing: string,
) {

const appRoot = path.resolve(projectRoot, appConfig.root);
Expand All @@ -46,6 +48,9 @@ export function getWebpackCommonConfig(
main: [appMain]
};

// determine hashing format
const hashFormat = getOutputHashFormat(outputHashing);

// process global scripts
if (appConfig.scripts && appConfig.scripts.length > 0) {
const globalScripts = extraEntryParser(appConfig.scripts, appRoot, 'scripts');
Expand Down Expand Up @@ -143,21 +148,31 @@ export function getWebpackCommonConfig(
entry: entryPoints,
output: {
path: path.resolve(projectRoot, appConfig.outDir),
publicPath: appConfig.deployUrl
publicPath: appConfig.deployUrl,
filename: `[name]${hashFormat.chunk}.bundle.js`,
sourceMapFilename: `[name]${hashFormat.chunk}.bundle.map`,
chunkFilename: `[id]${hashFormat.chunk}.chunk.js`
},
module: {
rules: [
{ enforce: 'pre', test: /\.js$/, loader: 'source-map-loader', exclude: [nodeModules] },

{ test: /\.json$/, loader: 'json-loader' },
{ test: /\.(jpg|png|gif)$/, loader: 'url-loader?limit=10000' },
{
test: /\.(jpg|png|gif)$/,
loader: `url-loader?name=[name]${hashFormat.file}.[ext]&limit=10000`
},
{ test: /\.html$/, loader: 'raw-loader' },

{ test: /\.(otf|ttf|woff|woff2)$/, loader: 'url-loader?limit=10000' },
{ test: /\.(eot|svg)$/, loader: 'file-loader' }
{
test: /\.(otf|ttf|woff|woff2)$/,
loader: `url-loader?name=[name]${hashFormat.file}.[ext]&limit=10000`
},
{ test: /\.(eot|svg)$/, loader: `file-loader?name=[name]${hashFormat.file}.[ext]` }
].concat(extraRules)
},
plugins: [
new ExtractTextPlugin(`[name]${hashFormat.extract}.bundle.css`),
new HtmlWebpackPlugin({
template: path.resolve(appRoot, appConfig.index),
filename: path.resolve(appConfig.outDir, appConfig.index),
Expand Down
13 changes: 1 addition & 12 deletions packages/angular-cli/models/webpack-build-development.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
const ExtractTextPlugin = require('extract-text-webpack-plugin');

export const getWebpackDevConfigPartial = function(projectRoot: string, appConfig: any) {
return {
output: {
filename: '[name].bundle.js',
sourceMapFilename: '[name].bundle.map',
chunkFilename: '[id].chunk.js'
},
plugins: [
new ExtractTextPlugin({filename: '[name].bundle.css'})
]
};
return { };
Copy link
Contributor

Choose a reason for hiding this comment

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

Gotta say I love the fact that this change makes the dev config dispensable. Can you remove it from the webpack merge procedure as well?

Copy link
Member Author

@clydin clydin Jan 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I was planning on making a PR that adds NamedModulesPluginto dev builds to support HMR. Can do it another way though (e.g., only when HMR is enabled), if you'd prefer the file removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there's a reason for the file to exist then it shouldn't be removed. I didn't know of NamedModulesPlugin but a quick google search made me think it's main use is to have good names in HMR, is that correct? If so, it makes sense to have it automatically with HMR, yes.

Copy link
Member Author

@clydin clydin Jan 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That's the reason I was going to add it. But it also stabilizes the module id's between builds which has benefits for cache busting. The hash won't change due to webpack module id renumbering. Nothing bad happens without it; the hash just might change more often than needed. And this isn't really relevant for HMR and probably not for dev builds in general.
For production builds there is HashedModuleIdsPlugin, which hashes the names to obfuscate the file path details.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be interested in such a PR then. Ping me if you end up doing it and I will review it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also wonder if that would help with the reload times tbh. It sounds like it might do away with rebuilding some modules due to id changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. But I'll see if I can put a PR together.
Also, I was looking through webpack-config.ts where the merging happens and to cleanly remove the dev config will require some refactoring (which the file could probably use either way) so it might be best to leave it for a followup PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to refactor the way we pass options down to the webpack config anyway, so I can do that at the time. It's getting messy as is.

};
7 changes: 0 additions & 7 deletions packages/angular-cli/models/webpack-build-production.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as path from 'path';
import * as webpack from 'webpack';
const ExtractTextPlugin = require('extract-text-webpack-plugin');
import {CompressionPlugin} from '../lib/webpack/compression-plugin';
const autoprefixer = require('autoprefixer');
const postcssDiscardComments = require('postcss-discard-comments');
Expand All @@ -22,13 +21,7 @@ export const getWebpackProdConfigPartial = function(projectRoot: string,
const appRoot = path.resolve(projectRoot, appConfig.root);

return {
output: {
filename: '[name].[chunkhash].bundle.js',
sourceMapFilename: '[name].[chunkhash].bundle.map',
chunkFilename: '[id].[chunkhash].chunk.js'
},
plugins: [
new ExtractTextPlugin('[name].[chunkhash].bundle.css'),
new webpack.DefinePlugin({
'process.env.NODE_ENV': JSON.stringify('production')
}),
Expand Down
18 changes: 18 additions & 0 deletions packages/angular-cli/models/webpack-build-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,21 @@ export function extraEntryParser(
return extraEntry;
});
}

export interface HashFormat {
chunk: string;
extract: string;
file: string;
}

export function getOutputHashFormat(option: string, length = 20): HashFormat {
/* tslint:disable:max-line-length */
const hashFormats: { [option: string]: HashFormat } = {
none: { chunk: '', extract: '', file: '' },
media: { chunk: '', extract: '', file: `.[hash:${length}]` },
bundles: { chunk: `.[chunkhash:${length}]`, extract: `.[contenthash:${length}]`, file: '' },
all: { chunk: `.[chunkhash:${length}]`, extract: `.[contenthash:${length}]`, file: `.[hash:${length}]` },
};
/* tslint:enable:max-line-length */
return hashFormats[option] || hashFormats['none'];
}
6 changes: 4 additions & 2 deletions packages/angular-cli/models/webpack-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export class NgCliWebpackConfig {
vendorChunk = false,
verbose = false,
progress = true,
deployUrl?: string
deployUrl?: string,
outputHashing?: string
) {
const config: CliConfig = CliConfig.fromProject();
const appConfig = config.config.apps[0];
Expand All @@ -48,7 +49,8 @@ export class NgCliWebpackConfig {
sourcemap,
vendorChunk,
verbose,
progress
progress,
outputHashing
);
let targetConfigPartial = this.getTargetConfig(
this.ngCliProject.root, appConfig, sourcemap, verbose
Expand Down
3 changes: 2 additions & 1 deletion packages/angular-cli/tasks/build-webpack-watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export default Task.extend({
runTaskOptions.vendorChunk,
runTaskOptions.verbose,
runTaskOptions.progress,
deployUrl
deployUrl,
runTaskOptions.outputHashing
).config;
const webpackCompiler: any = webpack(config);

Expand Down
3 changes: 2 additions & 1 deletion packages/angular-cli/tasks/build-webpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export default <any>Task.extend({
runTaskOptions.vendorChunk,
runTaskOptions.verbose,
runTaskOptions.progress,
deployUrl
deployUrl,
runTaskOptions.outputHashing
).config;

const webpackCompiler: any = webpack(config);
Expand Down
48 changes: 48 additions & 0 deletions tests/e2e/tests/build/output-hashing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import {stripIndents} from 'common-tags';
import * as fs from 'fs';
import {ng} from '../../utils/process';
import { writeMultipleFiles, expectFileToMatch } from '../../utils/fs';

function verifyMedia(css: RegExp, content: RegExp) {
return new Promise((resolve, reject) => {
const [fileName] = fs.readdirSync('./dist').filter(name => name.match(css));
if (!fileName) {
reject(new Error(`File ${fileName} was expected to exist but not found...`));
}
resolve(fileName);
})
.then(fileName => expectFileToMatch(`dist/${fileName}`, content));
}

export default function() {
return Promise.resolve()
.then(() => writeMultipleFiles({
'src/styles.css': stripIndents`
body { background-image: url("image.svg"); }
`,
'src/image.svg': 'I would like to be an image someday.'
}))
.then(() => ng('build', '--dev', '--output-hashing=all'))
.then(() => expectFileToMatch('dist/index.html', /inline\.[0-9a-f]{20}\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /main\.[0-9a-f]{20}\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /styles\.[0-9a-f]{20}\.bundle\.(css|js)/))
.then(() => verifyMedia(/styles\.[0-9a-f]{20}\.bundle\.(css|js)/, /image\.[0-9a-f]{20}\.svg/))

.then(() => ng('build', '--prod', '--output-hashing=none'))
.then(() => expectFileToMatch('dist/index.html', /inline\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /main\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /styles\.bundle\.(css|js)/))
.then(() => verifyMedia(/styles\.bundle\.(css|js)/, /image\.svg/))

.then(() => ng('build', '--dev', '--output-hashing=media'))
.then(() => expectFileToMatch('dist/index.html', /inline\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /main\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /styles\.bundle\.(css|js)/))
.then(() => verifyMedia(/styles\.bundle\.(css|js)/, /image\.[0-9a-f]{20}\.svg/))

.then(() => ng('build', '--dev', '--output-hashing=bundles'))
.then(() => expectFileToMatch('dist/index.html', /inline\.[0-9a-f]{20}\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /main\.[0-9a-f]{20}\.bundle\.js/))
.then(() => expectFileToMatch('dist/index.html', /styles\.[0-9a-f]{20}\.bundle\.(css|js)/))
.then(() => verifyMedia(/styles\.[0-9a-f]{20}\.bundle\.(css|js)/, /image\.svg/));
}