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.

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

Revision CSS & JS #270

Closed
wants to merge 1 commit into from
Closed

Revision CSS & JS #270

wants to merge 1 commit into from

Conversation

silvenon
Copy link
Member

@silvenon silvenon commented Feb 8, 2015

generator-webapp revisions all assets, I think, but gulp is not a suitable tool for these sorts of things, so CSS & JS will do for now.

Closes #255.

@silvenon
Copy link
Member Author

silvenon commented Feb 8, 2015

Forgot to update other recipes, just a sec…

@silvenon
Copy link
Member Author

silvenon commented Feb 8, 2015

Ok, ready.

@sindresorhus
Copy link
Member

Ugh, I'd rather we do not. It will lead to so many support issues about it not working in every edge case users have...

@silvenon
Copy link
Member Author

silvenon commented Feb 9, 2015

Ok, I don't have strong feelings either way. We could add it if we implement AssetGraph for the build step, it will do a much better job. Or you don't like revving by default in general?

@silvenon silvenon closed this Feb 9, 2015
@sindresorhus
Copy link
Member

Or you don't like revving by default in general?

I don't like it in general, as there are so many pitfalls and edge-cases to handle, but might be better with AssetGraph.

@austinpray
Copy link

Possibly semi-related question: Is there anything super egregious about just using the npm package version to version the assets? scripts/app.js?v={{ version }} -> scripts/app.js?v=1.1.0. On my current project I can't be bothered to deal with proper asset revving. Our CI only deploys to end-users if the package version changes anyway.

@silvenon
Copy link
Member Author

I’m not sure why you would update versions in package.json. Aren’t you only using it for installing packages?

@austinpray
Copy link

@silvenon Since we are already using npm for dev dependencies and runtime dependencies (browserify), we go whole hog and just use npm version (major, minor, patch) to keep track of our git tags. Not only is this consistent with our node application workflow, it has the added benefit of being able to use the version specified in package.json in our build process to revv assets. It also avoids this problem.

@silvenon
Copy link
Member Author

Hm, I think manually appending ?v={{ version }} to every asset (if I understood correctly) is very tedious. There needs to be a plugin that does this without the user having to think about it. AssetGraph is good at that, I just need to learn it well enough 😃

@austinpray
Copy link

@silvenon Well I am using browserify and not useref so I only have app.css and app.js to append version to. There is no difference between my dev build step from my prod build step except the exclusion of source maps.

Would like to pick you guys' brains about how I have things set up. I have used generator-gulp-webapp on close to 15 projects now and I am running into some of it's limitations. I'll collect my thoughts and perhaps make an issue with code examples.

@silvenon
Copy link
Member Author

I'm talking about images, fonts etc. All of those need to be revved too.

I'll collect my thoughts and perhaps make an issue with code examples.

Sure, we'd love to hear it! 😃

@JosefJezek
Copy link
Contributor

Do you know https://github.com/smysnk/gulp-rev-all?

@silvenon
Copy link
Member Author

Does it rename assets or do you end up with both original and revved files?

@JosefJezek
Copy link
Contributor

gulp-rev-all not only handles reference re-writing but it also takes child references into consideration when calculating a hashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add revision to static asset
4 participants