Sitelet https://github.com/nodejs/node/issues/2882
Skip to content

How to inherit from new Buffer implementation #2882

Description

@corentingurtner

Hi,
I would like to know how a class could inherit from the new Buffer implementation.

I'm trying to upgrade the node-ogg module for node v4 and nan v2.
(Here is my work so far)

I tried this classic inheritance model:

function ogg_packet (buffer) {
  if (!Buffer.isBuffer(buffer)) {
    Buffer.call(this, binding.sizeof_ogg_packet);
  } else {
    Buffer.call(this, buffer);
  }
  if (this.length != binding.sizeof_ogg_packet) {
    throw new Error('"buffer.length" = ' + this.length + ', expected ' + binding.sizeof_ogg_packet);
  }
}
inherits(ogg_packet, Buffer);

But I get this error when trying to access this.length :

Uncaught TypeError: Method Uint8Array.length called on incompatible receiver [object Object]

Thanks

Note: I also asked the question on SO:
http://stackoverflow.com/questions/32555714

Activity

  1. added
    bufferIssues and PRs related to the buffer subsystem.
    on Sep 15, 2015
  2. bnoordhuis commented on Sep 15, 2015

    @bnoordhuis
    Member

    ES6 extends should work:

    > class C extends Buffer { constructor(size) { super(size) } }
    [Function: C]
    
    > (new C(32)).length
    32
    
  3. corentingurtner commented on Sep 15, 2015

    @corentingurtner
    Author

    Thanks @bnoordhuis ,

    I know it can be seen as going backward, but is there a way in ES5 too?

  4. bnoordhuis commented on Sep 15, 2015

    @bnoordhuis
    Member

    I think that's going to be less easy for the same reason that it's difficult to inherit from Array or Uint8Array in ES5. You can maybe hack something together through the __proto__ property.

  5. trevnorris commented on Sep 15, 2015

    @trevnorris
    Contributor

    Try something like this:

    function Ogg() {
      const ui = new Uint8Array(size);
      Object.setPrototypeOf(ui, Ogg.prototype);
      return ui;
    }
    
    Ogg.prototype.__proto__ = Buffer.prototype;
    Ogg.__proto__ = Buffer;

    Yeah, ugly as sin.

  6. Fishrock123 commented on Sep 15, 2015

    @Fishrock123
    Contributor

    This should probably go into the buffer docs.

  7. added
    docIssues and PRs related to Node.js documentation.
    on Sep 15, 2015
  8. corentingurtner commented on Sep 15, 2015

    @corentingurtner
    Author

    Thanks a lot for your answers,

    @trevnorris :
    I succeed to create an instance this way thanks to your help:

    function ogg_packet (buffer) {
      if (!Buffer.isBuffer(buffer)) {
        buffer = new Uint8Array(binding.sizeof_ogg_packet);
      }
    
      if (buffer.length != binding.sizeof_ogg_packet) {
        throw new Error('"buffer.length" = ' + buffer.length + ', expected ' + binding.sizeof_ogg_packet);
      }
    
      Object.setPrototypeOf(buffer, ogg_packet.prototype);
      console.log('BUFFER', buffer.toString());
      return buffer;
    }
    ogg_packet.prototype.__proto__ = Buffer.prototype;
    ogg_packet.__proto__ = Buffer;

    However, it seems that I can't use Buffer methods (at least toString()) since there is a strict equality check of the instance. For example, trying to print buffer.toString() in the constructor function give me that error:

    Uncaught TypeError: argument should be a Buffer
     at TypeError (native)
     at ogg_packet.Buffer.toString (buffer.js:354:23)
    
  9. trevnorris commented on Sep 15, 2015

    @trevnorris
    Contributor

    @corentingurtner That would be from the node::Buffer::HasInstance() check. It's strictly checking the hidden class. Unfortunately there's no way I'm aware of to do proto transversal in the C++ API, similar to instanceof.

    /cc @domenic Know of a way to do this proto transversal check in C++?

    If there's not a quick fix then let's open a new issue for this.

  10. domenic commented on Sep 15, 2015

    @domenic
    Contributor

    @trevnorris you can use v8::Object::GetPrototype(). Something like objLocal->GetPrototype()->Get(context, v8_str("constructor")) to get the constructor and then check with Equals against the constructor.

  11. domenic commented on Sep 15, 2015

    @domenic
    Contributor

    What you really want though is to check that the appropriate internal data is set on the object. Which I think just would be val->IsUint8Array(). Otherwise I can fool it with crap like var notABuffer = {}; notABuffer.__proto__ = Buffer.prototype.

  12. trevnorris commented on Sep 15, 2015

    @trevnorris
    Contributor

    @domenic ah yup. sure enough. when I had to reimplement the check I was focused on not allowing users to bypass the instanceof check in JS.

    I agree that the check should be simplified down to checking internal data. All methods will work from that point regardless of whether it's actually a Buffer instance. I'll make a PR.

  13. self-assigned this
    on Sep 16, 2015
  14. ben-page commented on Sep 21, 2015

    @ben-page
    Contributor

    +1 This stops me from migrating to 4.x.

  15. trevnorris commented on Sep 24, 2015

    @trevnorris
    Contributor

    @bnoordhuis using classes won't work:

    'use strict';
    
    class Foo extends Buffer {
      constructor(n) { super(n); }
    
      foo() {
        let cntr = 0;
        for (let i = 0; i < this.length; i++) {
          cntr += this[i];
        }
        return cntr;
      }
    }
    
    let foo = new Foo(17).fill('abc');
    
    console.log(foo.foo());  // TypeError: foo.foo is not a function

    We are overwriting the prototype in the Buffer constructor, and loosing all the child's class methods. Want to examine this a bit more.

  16. domenic commented on Sep 24, 2015

    @domenic
    Contributor

    This is fixable once new.target ships in V8. You'd use new.target.prototype instead of Buffer.prototype.

  17. targos commented on Sep 24, 2015

    @targos
    Member

    We'll be able to fix this with new.target (only from V8 4.6):

    node [vee-eight-4.6●] % more test.js 
    'use strict';
    
    class A {
      constructor() {
        console.log(new.target);
      }
    }
    
    class B extends A {}
    
    new A();
    new B();
    
    node [vee-eight-4.6●] % ./node test
    [Function: A]
    [Function: B]
    
  18. trevnorris commented on Sep 24, 2015

    @trevnorris
    Contributor

    This still requires that HasInstance() basically becomes an IsUint8Array() check.

  19. trevnorris commented on Sep 24, 2015

    @trevnorris
    Contributor

    @domenic @targos Unless I'm missing something, new.target doesn't help class constructors:

    'use strict';
    
    class A {
      constructor() {
        if (!new.target)
          return { foo: 42 };
      }
    }
    
    let a = A();  // TypeError: Class constructors ...
  20. trevnorris commented on Sep 26, 2015

    @trevnorris
    Contributor

    @domenic The inconsistency must come from from mixing ES5 and ES6 style inheritance together. Take this example:

    class B extends Uint8Array { constructor() { super(0); } }
    class C extends B { constructor() { super(); } }
    
    let c = new C();
    
    console.log(c.constructor == C);
    console.log(c.__proto__ == C.prototype);
    console.log(c.__proto__.__proto__ == B.prototype);
    console.log(c.__proto__.__proto__.__proto__ == Uint8Array.prototype);

    But now doing the same thing with Buffer:

    class C extends Buffer { constructor() { super(0); } }
    
    let c = new C();
    
    console.log(c.constructor == Buffer);
    console.log(c.__proto__ == Buffer.prototype);
    console.log(c.__proto__.__proto__ == Uint8Array.prototype);

    As you can see it completely dropped anything related to C.

    What I'm missing is what new.target could help with.

  21. domenic commented on Sep 26, 2015

    @domenic
    Contributor

    @trevnorris right now allocate is doing Object.setPrototypeOf(ui8, Buffer.prototype), so even if someone does new C(), you are returning to them something with Buffer.prototype, not C.prototype.

    To fix this, you need to pass new.target from the Buffer constructor definition, to allocate, so that allocate can do Object.setPrototypeOf(ui8, newTarget.prototype).

    Then the returned object will have the correct prototype. In this case it would be C.prototype, since the target of new in new C() is C.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bufferIssues and PRs related to the buffer subsystem.docIssues and PRs related to Node.js documentation.questionIssues asking questions about Node.js.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions