generator-webapp icon indicating copy to clipboard operation
generator-webapp copied to clipboard

Add revision to static asset

Open gpedro opened this issue 10 years ago • 25 comments

References

gpedro avatar Jan 08 '15 14:01 gpedro

There is a recipe for this, but I think you're right, maybe we should implement this.

silvenon avatar Jan 09 '15 12:01 silvenon

in generator(grunt)-webapp it's by default https://github.com/yeoman/generator-webapp/blob/master/app/templates/Gruntfile.js#L255-L268

gpedro avatar Jan 09 '15 12:01 gpedro

Yeah, it would be nice if it was implemented here, too. The main problem is that I haven't figured out a way to rev all the assets, not just CSS & JS. Gulp is not very good at this stuff :disappointed: Otherwise I would implement it right away.

silvenon avatar Jan 09 '15 12:01 silvenon

I'm working on a solution using gulp-rev-all (see: https://github.com/smysnk/gulp-rev-all) It looks like it can rev all assets in a smart manner.

I'll make a pull-request once I have it successfully working on my local install, should be around this weekend

hivre avatar Mar 19 '15 01:03 hivre

Great! :+1:

silvenon avatar Mar 19 '15 08:03 silvenon

I have a first-setup working, but there's a bug that rev-all can't rewrite references in minified html files. Also, the current setup leaves revved and unrevved files in the dist (the latter should get removed)

A simple solution would be to move the html-minify line to a separate task, and use something like gulp-napkin to remove unrevved files.

But what I'd like to propose is a little bit more involved than the most simple solution:

  • split the html task in a dedicated html task and a separate minify task and minify-html task
  • the html-task at this moment would be rather empty (but you could add html-hint etc. it also makes extending it easier)
  • the minify task will do all (and only) minification, except html files.
  • write all minified files to /.tmp
  • build reads /.tmp and revs it, then minifies html files.

I prefer this as it separates .tmp and dist more, thus not needing an additional gulp-plugin (aside from the extendability that granular separate tasks provide). and the naming of the html-task is off, it only does minification atm. But I don't want to hijack this either, and there are probably a few other solutions available.

hivre avatar Mar 20 '15 00:03 hivre

Could you post an example of how would that html task look like?

silvenon avatar Mar 20 '15 00:03 silvenon

hmm, strike that task. I personally use it for aria and html linting, but I see that's my own addition...

So:

  • split the html task in ~~a dedicated html task and~~ a separate minify task and minify-html task

hivre avatar Mar 20 '15 06:03 hivre

If this would help revving, I'm all for it. But I think it's best to perform it on the dist folder after the build, that way we could rev fonts and images too. Or does splitting the html task make that easier/possible?

silvenon avatar Mar 20 '15 11:03 silvenon

My vision is:

start build -> copy[*]/minify() -> dest(.tmp) -> rev(.tmp) -> dest(dist) -> minifyHtml(dist)

That way the dist always has the latest and greatest, ready for deployment. The .tmp is a temporary dir where we collect everything before we rev it and copy it to dist. But I am also happy to see if gulp-napkin works as expected and use that to clean up the non-revved copies of files in dist, and leave the current situation as is.

[*] files we can't minify should just get copied.

hivre avatar Mar 20 '15 13:03 hivre

pull-request #293 uses the short-solution:

It uses gulp-rev-napkin to get rid of duplicate, un-revved files. This solution makes little changes to the gulp-file.

I have never done any testing -- I know, rather shamefull -- but I'm using this on my own project, and it works.

hivre avatar Mar 22 '15 19:03 hivre

Have you tried using the gulp-rev-replace plugin instead of gulp-rev-napkin? Just curious since the latter hasn't been updated in a while.

sondr3 avatar Mar 23 '15 11:03 sondr3

wasn't aware of gulp-rev-replace. I'll try it out asap.

hivre avatar Mar 23 '15 12:03 hivre

I tried out gulp-rev-replace, ~~but can't get it to work.~~ I just noticed a stupid mistake on my part, will try again ASAP.

Tbh. I can see rev-napkin not being updated if it works well. it's only 111 lines of code...

hivre avatar Mar 24 '15 06:03 hivre

I've got something with rev-replace running, but it actually removes the revved files, and leaves the un-revved files alone. Exactly the opposite we want :)

Might be my current implementation, I'm not using a manifest file. When I did that, I got an undefined error, most likely because the file doesn't exist (is in the stream) when it's used. Probably can get around this by creating a separate task, like in the rev-replace example. But that's precisely what I'm trying not to do.

hivre avatar Mar 24 '15 13:03 hivre

but it actually removes the revved files, and leaves the un-revved files alone

I was using it before and it worked fine. Maybe it gets screwed up when not replacing in the same stream as the one where revving is done?

silvenon avatar Mar 24 '15 13:03 silvenon

I've been using rev-replace for a while in my optimizing task and it does exactly what it should, removes the unrevved files and inserts references to the revved ones in the HTML. It should work.

sondr3 avatar Mar 24 '15 13:03 sondr3

@silvenon that's actually what I'm doing :) All in the same stream... @sondr3 rev-all already replaces the references. I use napkin to remove un-revved files only.

I should learn how to read:

 >  "Rewrite occurences of filenames which have been renamed by gulp-rev" 

We don't need that. gulp-rev-all does this for us. Since gulp-rev-all takes child-references in account for its hashing, it's objectively better than gulp-rev (and it saves a dependency, which is a nice bonus.)

Since we export all files to /dist, and then rev it, we get:

 |-index.html
 |-index.3432.html
 |-img
   |-logo.png
   |-logo.2343.png

gulp-napkin then removes the (duplicate) unrevved files.

 |-index.3432.html
 |-img
   |-logo.2343.png

Depending on the ignore list, it removes only the files that are actually revved. Thus in my defaults it ignores index.html in revving, gulp-napkin will leave index.html untouched.

However, like I said in an earlier post, if we export all files to /.tmp and then do gulp-rev-all and export to /dist we don't need gulp-napkin.

hivre avatar Mar 24 '15 13:03 hivre

But that's not really a problem if you properly do the tasks that will fix the assets, I never have non-revved assets in my dist-folder so gulp-napkin shouldn't be necessary at all. You need to take a look at the other tasks and maybe have them move their assets to a .tmp folder and move the revved files into the dist folder, that way you have no need for gulp-napkin.

Also, I thought that gulp-rev just renames the files itself but doesn't rename the references in the HTML, in other words you'll end up with the CSS that is being called still being named main.css while the one that you want is actually main-0b819as.css. Have you tried live serving your site after you've removed the non-revved files? AFAIK it should try to serve assets that do not exist since they reference the old files.

sondr3 avatar Mar 24 '15 16:03 sondr3

But that's not really a problem if you properly do the tasks that will fix the assets, I never have non-revved assets in my dist-folder so gulp-napkin shouldn't be necessary at all. You need to take a look at the other tasks and maybe have them move their assets to a .tmp folder and move the revved files into the dist folder, that way you have no need for gulp-napkin.

I will friendly refer you to my earlier message of 4 hours ago, which literally mentions this in the last sentence, or the message of 5 days ago where I mention this as well.

Also, I thought that gulp-rev just renames the files itself but doesn't rename the references in the HTML, in other words you'll end up with the CSS that is being called still being named main.css while the one that you want is actually main-0b819as.css.

I'm not using gulp-rev, but gulp-rev-all. gulp-rev-all takes child-references into account for its hashing, therefore it's better than gulp-rev (and it saves us from the dependency on gulp-rev-replace, which is a nice bonus.)

Have you tried live serving your site after you've removed the non-revved files? AFAIK it should try to serve assets that do not exist since they reference the old files.

I can recommend that you try it as well, you'll see how remarkably well it works :smile:

hivre avatar Mar 24 '15 17:03 hivre

Yes, I do realise that, I started writing my comment as I was reading and didn't spot it before I reached the end. I am also using gulp-rev-all but whenever I disable gulp-rev-replace it doesn't rewrite the references, I just tried and without using gulp-rev-replace none of the references were replaced. Weird, I never knew gulp-rev-all was supposed to do that, I'm going to take a look again. Sorry for being a bother :smiley:

sondr3 avatar Mar 24 '15 18:03 sondr3

I'm sorry for the non-response. I'm picking this up today

hivre avatar Apr 10 '15 07:04 hivre

Some news about that?

illycz avatar May 02 '17 20:05 illycz

@illycz no. 😕 We should really solve this, but Gulp isn't a good tool for this task.

silvenon avatar May 26 '17 12:05 silvenon