Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@VoltrexKeyva
Copy link
Member

The inherits() method in the util lib module is not using validators which others do use.

The `inherits()` method in the `util` lib module is not using validators which others do use.
@github-actions github-actions bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Jun 1, 2021
Removed the `ERR_INVALID_ARG_TYPE` constructor which was only used in the `inherits()` method which is replaced now.
The value must be a function to test the type of the prototype since validators first check if the value is a function.
The message expects to fail with the value `undefined`, not `null`.
@addaleax addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jun 1, 2021
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, because it makes the requirements on ctor and superCtor a bit stricter.

Since the docs already mention that one should use ES6 class extends instead, can we maybe just mark util.inherits with the new official "legacy" status instead?

@VoltrexKeyva
Copy link
Member Author

VoltrexKeyva commented Jun 1, 2021

This is a breaking change, because it makes the requirements on ctor and superCtor a bit stricter.

Since the docs already mention that one should use ES6 class extends instead, can we maybe just mark util.inherits with the new official "legacy" status instead?

I'm +1 on that, I would like to hear other collaborators/members opinions and see they agree as well 😄

@VoltrexKeyva
Copy link
Member Author

Just gonna open another PR referencing this to change the status to legacy, closing.

@VoltrexKeyva VoltrexKeyva deleted the patch-3 branch June 1, 2021 21:59
VoltrexKeyva added a commit to VoltrexKeyva/node that referenced this pull request Jun 1, 2021
aduh95 pushed a commit to VoltrexKeyva/node that referenced this pull request Jun 9, 2021
PR-URL: nodejs#38896
Refs: nodejs#38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
PR-URL: #38896
Refs: #38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
danielleadams pushed a commit that referenced this pull request Jun 17, 2021
PR-URL: #38896
Refs: #38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
richardlau pushed a commit that referenced this pull request Jul 19, 2021
PR-URL: #38896
Refs: #38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
PR-URL: #38896
Refs: #38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
PR-URL: nodejs#38896
Refs: nodejs#38893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.