gulpjs / vinyl

Virtual file format.
MIT License
1.28k stars 107 forks source link

Extending Vinyl still have build in properties #144

Closed juliet-zhu closed 5 years ago

juliet-zhu commented 5 years ago

Hi, in README.md [https://github.com/gulpjs/vinyl#extending-vinyl]

var Vinyl = require('vinyl');

var builtInProps = ['foo', '_foo'];

class SuperFile extends Vinyl {
  constructor(options) {
    super(options);
    this._foo = 'example internal read-only value';
  }

  get foo() {
    return this._foo;
  }

  static isCustomProp(name) {
    return super.isCustomProp(name) && builtInProps.indexOf(name) === -1;
  }
}
var superIns = new SuperFile();
var superInsCopy = superIns.clone();
console.log(superInsCopy.foo);

From my understanding, foo or _foo should not be in superIns and superInsCopy instacnes, but I still can query them.

phated commented 5 years ago

The documentation is phrased incorrectly. If something is set in the constructor, it'll be set on the clone because this.constructor() is called at https://github.com/gulpjs/vinyl/blob/master/index.js#L123 and anything on the prototype is also going to be there for the same reasons. The isCustomProp stuff is only to prevent someone from doing new Vinyl({ foo: "something" }) and overwrite a prop you assigned internally.

juliet-zhu commented 5 years ago

Much clear now. Thanks a lot.

phated commented 5 years ago

Glad that helped. I've updated the docs with my above explanation.