jonschlinkert / common-middleware

Common middleware for apps built with base-methods (like assemble, verb, generate, and update)
MIT License
6 stars 1 forks source link

TypeError: file.has is not a function #2

Open rbecheras opened 6 years ago

rbecheras commented 6 years ago

Got a TypeError parsing some files

[14:25:32] starting generate
[14:25:33] ✔ running tasks: [ 'swap-rest-api:dev' ]
[14:25:33] starting swap-rest-api
[14:25:33] starting swap-rest-api:dev task
[14:25:33] starting swap-rest-api:auto-prompt task
{ destination: '/home/remi/app/test-manual/test-manual' }
[14:25:33] finished swap-rest-api:auto-prompt task 1ms
[14:25:33] starting swap-rest-api:api task
[14:25:33] starting swap-rest-api.api
[14:25:33] starting swap-rest-api.api:default task
[14:25:33] starting swap-rest-api.api:api task
{ TypeError: file.has is not a function
    at stripPrefixes (/home/remi/app/node_modules/common-middleware/index.js:104:12)
    at /home/remi/app/node_modules/middleware-utils/index.js:42:9
    at next (/home/remi/app/node_modules/async-array-reduce/index.js:29:5)
    at /home/remi/app/node_modules/async-array-reduce/index.js:34:7
    at /home/remi/app/node_modules/middleware-utils/index.js:44:11
    at escape (/home/remi/app/node_modules/common-middleware/index.js:138:5)
    at /home/remi/app/node_modules/middleware-utils/index.js:42:9
    at next (/home/remi/app/node_modules/async-array-reduce/index.js:29:5)
    at /home/remi/app/node_modules/async-array-reduce/index.js:34:7
    at /home/remi/app/node_modules/middleware-utils/index.js:44:11
  _handled: true,
  source: 'at stripPrefixes (/home/remi/app/node_modules/common-middleware/index.js:104:12)',
  reason: 'generator#handle("onLoad"): /home/remi/app/dist/assets/templates/generate-api/src/assets/img/brand.png',
  file: <File "src/assets/img/brand.png" <Buffer 89 50 4e 47 0d 0a 1a 0a 00 00 00 0d 49 48 44 52 00 00 00 e9 00 00 00 51 08 06 00 00 01 40 a9 b1 6d 00 00 00 09 70 48 59 73 00 00 0b 13 00 00 0b 13 01 ... >> }

The error is thrown here: https://github.com/jonschlinkert/common-middleware/blob/master/index.js#L115

Here is a quick fix that solve the issue but we have to know why it happens. I don't know that for now.


/**
 * strip prefixes from dotfile and config templates
 */

function stripPrefixes(file, next) {
  if (!/templates/.test(file.dirname)) {
    next(null, file);
    return;
  }

  if (!file.prototype) {
    console.log(require('chalk').red('File is not a Vinyl object'), file)
    next(null, file)
    return
  }

  if (file.has && file.has('action.stripPrefixes')) {
    next(null, file);
    return;
  }

  file.set('action.stripPrefixes', true);
  file.basename = file.basename.replace(/^_/, '.');
  file.basename = file.basename.replace(/^\$/, '');
  next(null, file);
}
rbecheras commented 6 years ago

I forked the repo to publish a temporary fix:

rbecheras commented 6 years ago

@jonschlinkert I'll publish it as a PR tomorrow.

doowb commented 6 years ago

@rbecheras thanks! A PR would be great. Also, if you need to show that message in red, use ansi-colors or ansi-red directly.

rbecheras commented 6 years ago

@doowb ok for ansi-colors, noted.

But this error is weird. See the logged file that caused the error:

file: <File "src/assets/img/brand.png" <Buffer 89 50 4e 47 0d 0a 1a 0a 00 00 00 0d 49 48 44 52 00 00 00 e9 00 00 00 51 08 06 00 00 01 40 a9 b1 6d 00 00 00 09 70 48 59 73 00 00 0b 13 00 00 0b 13 01 ... >> }

It seems a vinyl file, it has got a path and a Buffer... But it has no longer a prototype !

Also it seems that the front-matter parsing middleware is responsible to pass a such "prototype-less" vinyl object to the next middleware, resulting in a TypeError on the first next statement that use any vinyl instance method. Here, file.has() in stripPrefixes().

I have to mention another important thing, probably the more important one. I have to make it really clear but it seems that the parsed files resulting in that error are files that doesn't have a front matter: assets like binary files or other templates without frontmatter.

Probably the best fix would be in the front matter middeware to completely pass on files that it doesnt matter ;-).

But it rest to find in code where the vinyl object misteriously loose its prototype...

However, a such Type checking in the stripPrefixes() function stays a good thing to keep. Don't you think ?

doowb commented 6 years ago

But it has no longer a prototype

That's because it's an instance of a Vinyl class. The class will have the prototype, but to check for it on an instance you can look for __proto__.

However, it seems like the error you're having is that .has is undefined. This will happen because the regular Vinyl file does not have a .has (or .set) method. That's something that was added somewhere else in the pipeline (probably by templates). We've been refactoring these things (templates, generate, assemble, etc...) to make it more obvious what's happening.

Probably the best fix would be in the front matter middeware to completely pass on files that it doesnt matter ;-).

The parse-front-matter plugin used here checks to see if the file is a binary file and if so, skips it. It won't modify the file contents. It also returns the original file object that was passed in.

However, a such Type checking in the stripPrefixes() function stays a good thing to keep. Don't you think ?

I think that the if (file.has && file.has('action. stripPrefixes') is a good thing. Also, maybe check for file.set before using it there.

rbecheras commented 6 years ago

Wow, I don't know where I loosed my head to confound the class and the instance... The fact that the methods are missing probably helped... Ok. I'll fix the PR later. (I have no longer battery now!)