Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: replace util.inherits() with es6 classes extends #6512

Closed
eljefedelrodeodeljefe opened this issue May 2, 2016 · 3 comments
Closed

doc: replace util.inherits() with es6 classes extends #6512

eljefedelrodeodeljefe opened this issue May 2, 2016 · 3 comments
Labels
doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.

Comments

@eljefedelrodeodeljefe
Copy link
Contributor

util.inherits() is probably long unloved. With full class support it would be worth thinking of slowly deprecating it as weakly suggested in #4179 by @bnoordhuis .

Since this would strongly promote the class keyword for the JS world we would need to think about whether we want to do this and in which form. Personally I am not a big fan of it in general - especially since es6 proposed style would have methods on a non 0 indentation level. So we could think about the following:

cc @nodejs/documentation @jasnell

const EventEmitter = require('events')

class PureES6 extends EventEmitter {
  constructor() {
    super()
  }

  echo(val) {
    console.log(val)
  }
}

let es6 = new PureES6()
es6.echo('A Value')


class Mixed extends EventEmitter {
  constructor() {
    super()
  }

  _echo(val) {
    console.log(val)
  }
}

Mixed.prototype.echo = function echo (val) {
  this._echo(val)
}

let mixed = new Mixed()
mixed.echo('A Value')
@eljefedelrodeodeljefe eljefedelrodeodeljefe added the doc Issues and PRs related to the documentations. label May 2, 2016
@mscdex mscdex added the util Issues and PRs related to the built-in util module. label May 2, 2016
@jasnell
Copy link
Member

jasnell commented May 2, 2016

util.inherits() is unfortunately going to be one of those warts that lives forever since it's so widely used. Unfortunately ES6 classes aren't much better ;-) ... it would likely be a good idea to begin slowly modernizing the examples in the docs but outright replacing likely isn't an option.

@eljefedelrodeodeljefe
Copy link
Contributor Author

Yep, sure :-(. One upside would be that extends supposedly keeps the prototype chain as discussed in #4179.
Do you have any preference in style? Having it on the first indentation or the second level?

@jasnell
Copy link
Member

jasnell commented May 2, 2016

This... always ;-)

class Foo extends Bar {
  constructor() {
  }

  baz() {
  }
}

eljefedelrodeodeljefe added a commit that referenced this issue May 9, 2016
util.inherits breaks the prototype chain. A fix does not seem
useful, since ES6 extends provides language level support for the
same functionality.

This commit starts fasing out mentions of the method.

Fixes: #6512
Fixes: #4179

PR-URL: #6514
Reviewed-By: James Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
evanlucas pushed a commit that referenced this issue May 17, 2016
util.inherits breaks the prototype chain. A fix does not seem
useful, since ES6 extends provides language level support for the
same functionality.

This commit starts fasing out mentions of the method.

Fixes: #6512
Fixes: #4179

PR-URL: #6514
Reviewed-By: James Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

3 participants