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

Docs: Add recipe for static asset revisioning #2499

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions docs/recipes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
* [Run Grunt Tasks from Gulp](run-grunt-tasks-from-gulp.md)
* [Rollup with rollup-stream](rollup-with-rollup-stream.md)
* [Run gulp task via cron job](cron-task.md)
* [Sass and Nunjucks with static asset revisioning](sass-and-nunjucks-with-static-asset-revisioning.md)
79 changes: 79 additions & 0 deletions docs/recipes/sass-and-nunjucks-with-static-asset-revisioning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Sass and Nunjucks with static asset revisioning

Static asset revisioning by appending content hash to filenames. Make sure to set the files to [never expire](http://developer.yahoo.com/performance/rules.html#expires) for this to have an effect.

Sass and Nunjucks are here just to demonstrate how to implement static asset revisioning in a more real-world scenario. The content has will be appended in production, so it's recommended to add the following npm scripts to `package.json`:

```json
{
"scripts": {
"dev": "gulp-dev",
Copy link
Member

Choose a reason for hiding this comment

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

If we just implement 1 task, this can be NODE_ENV=development gulp build

"build": "NODE_ENV=production gulp build"
}
}
```

Now to install the required dependencies:

```sh
npm install --save-dev gulp gulp-rev gulp-rev-rewrite gulp-sass gulp-nunjucks gulp-if del
```

And, finally, `gulpfile.js`:

```js
const gulp = require('gulp')
const sass = require('gulp-sass')
const nunjucks = require('gulp-nunjucks')
const rev = require('gulp-rev')
const revRewrite = require('gulp-rev-rewrite')
const gulpIf = require('gulp-if')
const del = require('del')

const isProd = process.env.NODE_ENV === 'production'

function clean() {
return del('dist')
}
Comment on lines +35 to +37
Copy link
Member

Choose a reason for hiding this comment

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

If you are using static revisioning, I'm pretty sure you're never supposed to remove your old files (in case someone loads an old version from their cache).

Copy link
Contributor Author

@silvenon silvenon Oct 22, 2020

Choose a reason for hiding this comment

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

Oh… I thought that if an old version is being loaded, then static revisioning isn't set up correctly because the cache was supposed to be busted. That's a terrifying thought for me as an author that someone loads an old CSS or JS file, I guess things will break for them either way? 🤔 Also, what about old pages? Isn't showing a new 404 page better than showing an old page that has been deleted in the source?


function styles() {
return gulp
Copy link
Member

Choose a reason for hiding this comment

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

Let's use pipeline() (from node's Stream module) syntax any time things are being piped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you be more specific? I never used it before.

.src('styles/style.scss')
.pipe(sass().on('error', sass.logError))
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if this pattern even makes sense anymore with pipeline()

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 would like to avoid this, too, but, as I said, I need help. 🙏

// appends the content hash
.pipe(gulpIf(isProd, rev()))
.pipe(gulp.dest('dist'))
// output rev-manifest.json
.pipe(gulpIf(isProd, rev.manifest()))
.pipe(gulp.dest('dist'))
}

function views() {
const manifest = gulp.src(
'dist/rev-manifest.json',
// in development the manifest doesn't exist
{ allowEmpty: !isProd }
)
return gulp
.src('views/**/*.njk')
.pipe(nunjucks.compile({ title: 'Hello world!' }))
// this updates the reference(s) to revisioned assets
.pipe(gulpIf(isProd, revRewrite({ manifest })))
Copy link
Member

Choose a reason for hiding this comment

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

Wow, this manifest option is a terrible API that's not using gulp correctly. I think you should combine views and styles tasks into 1 pipeline and use gulp-filter to manage the "phases".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll give it a shot.

.pipe(gulp.dest('dist'))
}

function watch() {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just remove the watch stuff, it doesn't add anything to this recipe.

Copy link
Contributor Author

@silvenon silvenon Oct 22, 2020

Choose a reason for hiding this comment

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

Also, nothing happens because I forgot to add tasks to them, so they are completely useless anyway 😆

gulp.watch('styles/**/*.scss')
gulp.watch('views/**/*.njk')
}

// after everything is done, we no longer need the manifest file
function deleteRevManifest() {
return del('dist/rev-manifest.json')
}
Comment on lines +70 to +73
Copy link
Member

Choose a reason for hiding this comment

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

If you combine the 2 tasks, we no longer need to delete some intermediary file on the filesystem.


exports.dev = gulp.series(clean, gulp.parallel(styles, views), watch)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should have a separate build and dev script. You are already vary on isProd which should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we're removing watching (which I will), then it doesn't.

// in build it's important that "views" runs AFTER "styles"
// if it runs before, the manifest file won't exist yet
Comment on lines +76 to +77
Copy link
Member

Choose a reason for hiding this comment

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

If we change to my suggestions above, you don't need this not anymore since everything will be streaming within one task.

exports.build = gulp.series(clean, styles, views, deleteRevManifest)
```