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

clydin · 2017-01-06T19:14:32Z

This allows the output filename hashes to be configured during a build via a new build command option --output-hashing. There are four possible values:

none: no hashing performed
media: only add hashes to files processed via [url|file]-loaders
bundles: only add hashes to the output bundles
all: add hashes to both media and bundles

none is the default for the development target.
all is the default for the production target.

Fix #2774

aegyed91 · 2017-01-06T20:25:42Z

packages/angular-cli/models/webpack-build-utils.ts

+
+const hashFormats: { [option: string]: any } = {
+  'none':     { chunk: '',             extract: '',                  file: '[name]'    },
+  'media':    { chunk: '',             extract: '',                  file: '[hash:20]' },


hashlength note: I think a 20 character long hash is unnecessary and make it much more less human readable. 6 would be much better.

media (file) template format note: as stated in #3871 (comment). sky-bg.jpg becomes 8bcfb98e274f484ebac02d81670b7e4c.jpg with '[hash:20]'. Don't you think [name].[hash:20] would be more ideal? Looking at 8bcfb98e274f484ebac02d81670b7e4c.jpg have absolutely no idea which image it is, that is a problem is think.

20 is the webpack default and the existing tests rely on it so I went with it for consistency.
Personally I'd prefer a small decrease in human readability in exchange for a lower chance of collision. Especially considering they're not intended to be overly human readable. (we have dev builds for that.)
How about something in the middle: 12?
You're example is actually [hash:32] or just [hash] when using the file-loader. Oddly, [hash] means different things in different places in webpack and the defaults don't seem to carry over either.
I agree on the name inclusion as it would be consistent with the others but was hesitant to change the existing paradigm without a consensus.

@filipesilva thoughts?

How about something in the middle: 12?

Sounds good to me if you dont mind also editing the test cases.

I agree on the name inclusion as it would be consistent with the others but was hesitant to change the existing paradigm without a consensus.

Lets get a green light from one of the maintainers. File- and url-loader should emit a filename containing [name].[hash:12] or [name].[hash:20] for consistency and human readability.

Regarding hash size, I think it's better to be consistent with the rest of the hashes already present so 20 sounds good to me.

The question of including the name is less obvious for me though. Having the name would be a change in the previous behaviour, but if we look at it with fresh eyes I think it's simply a better option since now everything that's processed follows the same pattern of [name].[hash].ext.

Some users are confused by the multitude of files suddenly propping up in their dist folder and that would help them make sense of it. It also adds a bit more collision protection.

So my vote goes for [name].[hash:20]. I think [name] should be in the common config instead of here though (like the others).

filipesilva

Tests are missing as you mentioned but I think this is a really good PR.

It adds functionality users have been asking for, expands upon the fix itself to offer granular support and eliminates code duplication we had before due to hardcoded values.

Great work @clydin and @tsm91!

PS.: I hope you don't mind but I edited the PR comment to include the issue this PR fixes.

filipesilva · 2017-01-07T17:04:30Z

packages/angular-cli/models/webpack-build-development.ts

-      new ExtractTextPlugin({filename: '[name].bundle.css'})
-    ]
-  };
+  return { };


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?

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.

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.

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.

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

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.

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.

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.

filipesilva · 2017-01-07T17:14:11Z

packages/angular-cli/models/webpack-build-utils.ts

+
+const hashFormats: { [option: string]: any } = {
+  'none':     { chunk: '',             extract: '',                  file: '[name]'    },
+  'media':    { chunk: '',             extract: '',                  file: '[hash:20]' },


Regarding hash size, I think it's better to be consistent with the rest of the hashes already present so 20 sounds good to me.

The question of including the name is less obvious for me though. Having the name would be a change in the previous behaviour, but if we look at it with fresh eyes I think it's simply a better option since now everything that's processed follows the same pattern of [name].[hash].ext.

Some users are confused by the multitude of files suddenly propping up in their dist folder and that would help them make sense of it. It also adds a bit more collision protection.

So my vote goes for [name].[hash:20]. I think [name] should be in the common config instead of here though (like the others).

filipesilva · 2017-01-07T17:27:10Z

Regarding #3853, it actually asks for something slightly different. That request is for scripts/styles marked as lazy to never have hashes.

Personally I don't know of a way to exclude some files from having the hash, nor think it needs to be together in this PR. But if any of you know a way to do it I'd appreciate being pointed in the right direction and then I can make a followup PR with that fix (or you can, if you want to).

aegyed91 · 2017-01-09T21:27:14Z

@filipesilva im not sure how this lazy option works in angular-cli, wanted to search the readme.md but got 0 results for the word lazy :D. Only do lazy route module loads with loadChildren, but that is a different story.

What i know is the AOT build output files will get hashed or non hashed the same way. It doesnt matter if its an AOT or non AOT build, what matters is if it is a dev or prod environment build: https://github.com/angular/angular-cli/blob/master/packages/angular-cli/models/webpack-config.ts#L53-L58

Just did an aot build on a fork with cache-bust disabled ng build --aot --prod --no-sourcemap --verbose --dcb and got the desired output: http://i.imgur.com/prcUbG0.png . Ofc in my fork the url-, file-loader still generates hashed media files.

.. nor think it needs to be together in this PR

I think this PR should be merged and get into beta.25.

filipesilva · 2017-01-10T15:13:18Z

@tsm91 that lazy loaded scripts/styles functionality is actually a different thing than lazy loaded routes. See #3401 for details about them.

aegyed91 · 2017-01-10T15:24:16Z

@filipesilva https://github.com/angular/angular-cli/pull/3402/files i wonder why there is no README.md file in the changes. This feature is not even documented.

filipesilva · 2017-01-10T20:16:59Z

@tsm91 I didn't add them at the time, which was really a bad choice.

angular-automatic-lock-bot · 2019-09-11T18:42:01Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

feat: allow output hashing to be configured

47820b8

googlebot added the cla: yes label Jan 6, 2017

clydin mentioned this pull request Jan 6, 2017

feat(build): add ability to disable webpack's cache busting mechanism #3871

Closed

aegyed91 reviewed Jan 6, 2017

View reviewed changes

filipesilva suggested changes Jan 7, 2017

View reviewed changes

cleanup and address comments

e8bb961

add tests

0690808

clydin changed the title ~~[WIP]feat: allow output hashing to be configured~~ feat(build): allow output hashing to be configured Jan 10, 2017

filipesilva approved these changes Jan 10, 2017

View reviewed changes

filipesilva added the pr_action: merge label Jan 10, 2017

add docs

90d3e7e

hansl merged commit b82fe41 into angular:master Jan 10, 2017

clydin deleted the output-hash branch January 11, 2017 01:12

Meligy mentioned this pull request Jan 13, 2017

image assets cache purge question #3959

Closed

filipesilva mentioned this pull request Jan 16, 2017

ng github-page:deploy not generating files with hashes, only main.bundle.js #4048

Closed

MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017

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

8542b26

dmesa mentioned this pull request Feb 12, 2017

Removing hash from Webpack bundle #1833

Closed

agermanique mentioned this pull request Nov 6, 2017

Unique identifier ng build --prod #3769

Closed

k11k2 mentioned this pull request Feb 26, 2018

Question: What are others approach for SPA and MVC to trigger authentication? aspnet/JavaScriptServices#1552

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

clydin commented Jan 6, 2017 •

edited

Loading

aegyed91 Jan 6, 2017

clydin Jan 7, 2017 •

edited

Loading

aegyed91 Jan 7, 2017

filipesilva Jan 7, 2017

filipesilva left a comment

filipesilva Jan 7, 2017

clydin Jan 7, 2017 •

edited

Loading

filipesilva Jan 7, 2017

clydin Jan 7, 2017 •

edited

Loading

filipesilva Jan 7, 2017

filipesilva Jan 7, 2017

clydin Jan 7, 2017

filipesilva Jan 7, 2017

filipesilva Jan 7, 2017

filipesilva commented Jan 7, 2017

aegyed91 commented Jan 9, 2017 •

edited

Loading

filipesilva commented Jan 10, 2017

aegyed91 commented Jan 10, 2017

filipesilva commented Jan 10, 2017

angular-automatic-lock-bot bot commented Sep 11, 2019

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

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

Conversation

clydin commented Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

clydin Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipesilva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clydin Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clydin Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipesilva commented Jan 7, 2017

aegyed91 commented Jan 9, 2017 • edited Loading

filipesilva commented Jan 10, 2017

aegyed91 commented Jan 10, 2017

filipesilva commented Jan 10, 2017

angular-automatic-lock-bot bot commented Sep 11, 2019

clydin commented Jan 6, 2017 •

edited

Loading

clydin Jan 7, 2017 •

edited

Loading

clydin Jan 7, 2017 •

edited

Loading

clydin Jan 7, 2017 •

edited

Loading

aegyed91 commented Jan 9, 2017 •

edited

Loading