Always create rev-manifest.json even if no -rev #40

Closed
kud opened this Issue Jul 16, 2014 · 12 comments

Comments

4 participants

kud commented Jul 16, 2014

Is it possible to create rev-manifest.json with:

{
  "style.css": "style.css"
}

even if you don't want to rev your files? It allows me in backend to always read rev-manifest.json and to include what I need/want from frontend.

@kud kud changed the title from Create rev-manifest.json even if no -rev to Always created rev-manifest.json even if no -rev Jul 16, 2014

@kud kud changed the title from Always created rev-manifest.json even if no -rev to Always create rev-manifest.json even if no -rev Jul 16, 2014

Collaborator

bobthecow commented Jul 16, 2014

The problem you'll run into is that non-rev'd assets don't show up in the manifest even when you do have rev'd assets. A better approach is probably to handle it on the backend...

My backend defaults to returning the original filename if it doesn't find it in the manifest. That way it always works with non-rev'd assets, and in development when i'm not rev'ing any assets.

kud commented Jul 16, 2014

It's exactly what we currently do and it's a mistake in my opinion.

The backend shouldn't know the name of the files which must be included.

It should be like that:

<html>
<link src="{helper('style.css')}" />
</html>

and you've got in the helper, it reads the rev-manifest.json, points on style.css, and read the real file name, which can be style.css or style-ezfd34dfg4.css.

Collaborator

bobthecow commented Jul 16, 2014

But the backend just asked for it by name... All I'm saying is to default to that name if you don't have a "real" file name:

helper = function (name) {
  return manifest[name] || name;
}

kud commented Jul 16, 2014

I understand your point and it's probably a solution, but it's like respecting a contract. You always need the contract, even if there's no change. Thinking about what if the files exists or not, it's like thinking about respecting or not the contract in my opinion.

Collaborator

bobthecow commented Jul 16, 2014

I can see why you'd think of it like that, but I see it the exact opposite :)

Your app says "I need styles.css". By default, that's styles.css, because that's what you asked for. But gulp-rev offers you a better version: styles-HASH.css. If gulp-rev never gave you that file, you'd still have styles.css; it's the baseline.

I see it more like i18n functions. You give it an english string, and if there's a translation available your i18n implementation hands you back the translation. Otherwise, there's always the english string.

kud commented Jul 16, 2014

What about an option so? O:)

gyndav commented Jul 16, 2014

The point is implementing a kind of helper within a template engine (think of Smarty for instance) and honestly, that may be a good way out, except you don't want to maintain that piece of code.

Owner

sindresorhus commented Jul 18, 2014

I agree with @bobthecow. We're not interested in adding an option for this, but you're free to fork if gulp-rev doesn't work for you :)

kud commented Jul 18, 2014

Thank you to be so open-minded. ;)

Have a good weekend.

(Note : please, do not complain there're soooooo many node modules which do exactly the same thing.)

Collaborator

bobthecow commented Jul 18, 2014

@kud It's not very gulpy to have options. If you want a fake manifest (i.e. one without hashes) you can easily write a function to provide that:

var fakeManifest = function () {
  var manifest  = {};
  var firstFile = null;
  var basePath  = null;

  function relPath(base, filePath) {
    if (filePath.indexOf(base) !== 0) {
      return filePath;
    }
    var newPath = filePath.substr(base.length);
    if (newPath[0] === path.sep) {
      return newPath.substr(1);
    } else {
      return newPath;
    }
  }

  return through.obj(function (file, enc, cb) {
    if (file.path) {
      if (!firstFile) {
        firstFile = file
        basePath  = firstFile.revOrigBase || firstFile.base;
      }

      var fname = relPath(pasePath, file.path);
      if (file.revOrigPath) {
        manifest[relPath(basePath, file.revOrigPath)] = fname;
      } else {
        manifest[fname] = fname;
      }
    }

    cb();
  }, function (cb) {
    if (firstFile) {
      this.push(new gutil.File({
        cwd: firstFile.cwd,
        base: firstFile.base,
        path: path.join(firstFile.base, 'rev-manifest.json'),
        contents: new Buffer(JSON.stringify(manifest, null, '  '))
      }));
    }

    cb();
  });
};

You're right, this could be included in gulp-rev as an option, but that would double the amount of logic in the plugin, for a use case that (so far) only one person wants. You're not wrong for wanting it, you're just wrong for wanting it shoehorned into gulp-rev ;)

As I said, adding it would not be very gulp-like:

https://github.com/gulpjs/gulp/blob/master/docs/writing-a-plugin/guidelines.md

kud commented Jul 18, 2014

This is a more appropriated answer. Clear, well explained. Thank you
Justin. ;)
On 18 Jul 2014 18:44, "Justin Hileman" notifications@github.com wrote:

@kud https://github.com/kud It's not very gulpy to have options. If you
want a fake manifest (i.e. one without hashes) you can easily write a
function to provide that:

var fakeManifest = function () {
var manifest = {};
var firstFile = null;
var basePath = null;

function relPath(base, filePath) {
if (filePath.indexOf(base) !== 0) {
return filePath;
}
var newPath = filePath.substr(base.length);
if (newPath[0] === path.sep) {
return newPath.substr(1);
} else {
return newPath;
}
}

return through.obj(function (file, enc, cb) {
// ignore all non-rev'd files
if (file.path) {
if (!firstFile) {
firstFile = file
basePath = firstFile.revOrigBase || firstFile.base;
}

  var fname = relPath(pasePath, file.path);
  if (file.revOrigPath) {
    manifest[relPath(basePath, file.revOrigPath)] = fname;
  } else {
    manifest[fname] = fname;
  }
}

cb();

}, function (cb) {
if (firstFile) {
this.push(new gutil.File({
cwd: firstFile.cwd,
base: firstFile.base,
path: path.join(firstFile.base, 'rev-manifest.json'),
contents: new Buffer(JSON.stringify(manifest, null, ' '))
}));
}

cb();

});};

You're right, this could be included in gulp-rev as an option, but that
would double the amount of logic in the plugin, for a use case that (so
far) only one person wants. You're not wrong for wanting it, you're just
wrong for wanting it shoehorned into gulp-rev ;)

As I said, adding it would not be very gulp-like:

https://github.com/gulpjs/gulp/blob/master/docs/writing-a-plugin/guidelines.md


Reply to this email directly or view it on GitHub
#40 (comment)
.

kud commented Sep 19, 2014

I did that:

.pipe(
      gutil.env.dist ?
      require('gulp-rev').manifest() :
      require('through2')
        .obj( function ( file, enc, cb ) {
          var tmp = file.path.split('/')
          tmp = tmp.slice( tmp.length - 1, tmp.length )[0]
          manifest[ tmp ] = tmp

          cb()
        })
        .on('end', function() {
          require('fs').createWriteStream('web/assets/rev-manifest.json').end( JSON.stringify( manifest, null, 2 ) )
        })
    )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment