Docs: Add recipe for static asset revisioning #2499
Conversation
silvenon
force-pushed
the
docs/recipe-asset-revisioning
branch
from
8d4d619
to
d905115
Compare
silvenon
force-pushed
the
docs/recipe-asset-revisioning
branch
from
d905115
to
86859db
Compare
|
How about this approach? |
| } | ||
|
|
||
| function styles() { | ||
| return gulp |
There was a problem hiding this comment.
Let's use pipeline() (from node's Stream module) syntax any time things are being piped.
There was a problem hiding this comment.
Could you be more specific? I never used it before.
| return del('dist/rev-manifest.json') | ||
| } | ||
|
|
||
| exports.dev = gulp.series(clean, gulp.parallel(styles, views), watch) |
There was a problem hiding this comment.
I don't think we should have a separate build and dev script. You are already vary on isProd which should be enough.
There was a problem hiding this comment.
If we're removing watching (which I will), then it doesn't.
| .src('views/**/*.njk') | ||
| .pipe(nunjucks.compile({ title: 'Hello world!' })) | ||
| // this updates the reference(s) to revisioned assets | ||
| .pipe(gulpIf(isProd, revRewrite({ manifest }))) |
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
Ok, I'll give it a shot.
| // after everything is done, we no longer need the manifest file | ||
| function deleteRevManifest() { | ||
| return del('dist/rev-manifest.json') | ||
| } |
There was a problem hiding this comment.
If you combine the 2 tasks, we no longer need to delete some intermediary file on the filesystem.
| .pipe(gulp.dest('dist')) | ||
| } | ||
|
|
||
| function watch() { |
There was a problem hiding this comment.
I think we should just remove the watch stuff, it doesn't add anything to this recipe.
There was a problem hiding this comment.
Also, nothing happens because I forgot to add tasks to them, so they are completely useless anyway 😆
| function styles() { | ||
| return gulp | ||
| .src('styles/style.scss') | ||
| .pipe(sass().on('error', sass.logError)) |
There was a problem hiding this comment.
I'm wondering if this pattern even makes sense anymore with pipeline()
There was a problem hiding this comment.
I would like to avoid this, too, but, as I said, I need help. 🙏
| function clean() { | ||
| return del('dist') | ||
| } |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
| ```json | ||
| { | ||
| "scripts": { | ||
| "dev": "gulp-dev", |
There was a problem hiding this comment.
If we just implement 1 task, this can be NODE_ENV=development gulp build
| // in build it's important that "views" runs AFTER "styles" | ||
| // if it runs before, the manifest file won't exist yet |
There was a problem hiding this comment.
If we change to my suggestions above, you don't need this not anymore since everything will be streaming within one task.
|
I'm learning (more) about streams in Node.js now so I can finish this PR. I actually never studied them before, I somehow managed to get by in gulp without it, but not having this theory established is becoming pretty uncomfortable. 😄 |
|
If you have some good resources, I'm all ears. 👂 |
Even though this was not requested in #2164, it's a common practice, so a recipe would be nice. 😃