nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.03k stars 29.29k forks source link

[Buffer + ES6] Lose the child's class method when extending Buffer in ES6 class style #3739

Closed hashedhyphen closed 8 years ago

hashedhyphen commented 8 years ago

This issue is related to #2882 .

I'm now using a nightly version.

>node -v
v5.0.1-nightly201510294e54dbec51

As @trevnorris commented on 25 Sep, a following code doesn't work well now.

'use strict';

class Child extends Buffer {
  constructor(n) {
    super(n);
  }

  foo() {
    console.log('gotcha!');
  }
}

let child = new Child(4);
child.foo();  //=> TypeError: child.foo is not a function

I'm not sure how difficult is this problem to fix, but I would be happy if the above code works...

Thanks! :)

trevnorris commented 8 years ago

Just wrote a conceptual patch to get this working, but haven't been able to remove the performance hit from the current case. Will continue to work on this.

hashedhyphen commented 8 years ago

@trevnorris Thank you for your quick responce! I'm looking forward to reflecting the patch to a stable version <3

trevnorris commented 8 years ago

For reference, here's the patch and branch of the conceptual fix: https://github.com/trevnorris/node/commit/c1bc39426d75242da156901a21971c820e2dc686

hashedhyphen commented 8 years ago

@trevnorris It is surprising to me that where to fix is JavaScript code. Just looked over c1bc39426d75242da156901a21971c820e2dc686, I wonder if perhaps Object.setPrototypeOf slows the performance as noted in MDN, but I'm not sure...

trevnorris commented 8 years ago

@hashedhyphen The use of Object.setPrototypeOf() is definitely a hit, but we're incurring that cost regardless (since it must be used on Uint8Array() instance). The extra cost comes from accessing new.target. Though my implementation is the first thing that popped out (took most the effort figuring out all the places where it needed to happen) and can probably be easily made faster.

hashedhyphen commented 8 years ago

@trevnorris I see. Thanks to your kindness, I was able to learn Node's inside more!

jasnell commented 8 years ago

Assuming there's no further reason to keep this one open. Closing!

techtenk commented 7 years ago

Did the proposed fix above ever go in? I'm running into the same problem on Node v7.10.0

Tims-MBP:nodesandbox techten$ cat buffertest.js
'use strict';

class Child extends Buffer {
  constructor(n) {
    super(n);
  }

  foo() {
    console.log('gotcha!');
  }
}

let child = new Child(4);
child.foo();
Tims-MBP:nodesandbox techten$ node buffertest.js
/Users/techten/Documents/nodesandbox/buffertest.js:14
child.foo();
      ^

TypeError: child.foo is not a function
    at Object.<anonymous> (/Users/techten/Documents/nodesandbox/buffertest.js:14:7)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:10)
    at run (bootstrap_node.js:427:7)
    at startup (bootstrap_node.js:151:9)
    at bootstrap_node.js:542:3
Tims-MBP:nodesandbox techten$ node -v
v7.10.0