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

util: fix module inspection & instanceof check during inspect #36178

Closed
wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Nov 19, 2020

Fixes: #36151
Fixes: #35730
Refs: #35754

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Nov 19, 2020
@ExE-Boss
Copy link
Contributor

ExE-Boss commented Nov 19, 2020

This PR should also have the following in the PR description and the message of commit d330cd1:

Supersedes and closes #35754

@BridgeAR
Copy link
Member Author

@ExE-Boss I added a reference to the issue description but I'd just close the PR manually when this PR lands.

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Actual fix LGTM (pending fixed messages changed in tests probably?)

Also - what semverness should this have?

@ExE-Boss
Copy link
Contributor

@benjamingr

Also - what semverness should this have?

Probably semver‑patch and be backported to v14.x, given that #33449 wasn’t a semver‑major change either.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730
@BridgeAR BridgeAR added the needs-ci PRs that need a full CI run. label Nov 20, 2020
@@ -534,6 +534,14 @@ function getEmptyFormatArray() {
return [];
}

function isInstanceof(object, proto) {
try {
return object instanceof proto;
Copy link
Member

Choose a reason for hiding this comment

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

TIL: instanceof can throw

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 20, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 20, 2020
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Thanks so much for fixing this up. Really nice to surface the null prototype too.

@richardlau richardlau added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Dec 7, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2020
@nodejs-github-bot
Copy link
Collaborator

@ExE-Boss
Copy link
Contributor

The node‑test‑linux‑linked‑debug Jenkins CI build needs restarting.

@nodejs-github-bot
Copy link
Collaborator

@ExE-Boss
Copy link
Contributor

node‑test‑linux‑linked‑debug failed again.

@BridgeAR
Copy link
Member Author

@ExE-Boss the failure is an old run and not properly updated :)

@BridgeAR BridgeAR added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 12, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 12, 2020
@github-actions
Copy link
Contributor

Landed in 738cd60...1e66509

@github-actions github-actions bot closed this Dec 12, 2020
nodejs-github-bot pushed a commit that referenced this pull request Dec 12, 2020
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: #36151

PR-URL: #36178
Fixes: #35730
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Dec 12, 2020
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: #35730

PR-URL: #36178
Fixes: #36151
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Dec 21, 2020
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: #36151

PR-URL: #36178
Fixes: #35730
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Dec 21, 2020
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: #35730

PR-URL: #36178
Fixes: #36151
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Pezmc
Copy link

Pezmc commented Jan 27, 2021

Is this likely to be backported to 14.x LTS? It doesn't seem to be included in 14.15.3 @ExE-Boss

Edit: trying to follow https://github.com/nodejs/node/blob/master/doc/guides/backporting-to-release-lines.md myself!

Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 27, 2021
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151

PR-URL: nodejs#36178
Fixes: nodejs#35730
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 27, 2021
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730

PR-URL: nodejs#36178
Fixes: nodejs#36151
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 27, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151

PR-URL: nodejs#36178
Fixes: nodejs#35730
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 27, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730

PR-URL: nodejs#36178
Fixes: nodejs#36151
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151

PR-URL: nodejs#36178
Fixes: nodejs#35730
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730

PR-URL: nodejs#36178
Fixes: nodejs#36151
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151

PR-URL: nodejs#36178
Fixes: nodejs#35730
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730

PR-URL: nodejs#36178
Fixes: nodejs#36151
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#36151

PR-URL: nodejs#36178
Fixes: nodejs#35730
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pezmc pushed a commit to Pezmc/node that referenced this pull request Jan 29, 2021
Backport-PR-URL: nodejs#37100

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Fixes: nodejs#35730

PR-URL: nodejs#36178
Fixes: nodejs#36151
Refs: nodejs#35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Backport-PR-URL: #37100
PR-URL: #36178
Fixes: #35730
Fixes: #36151
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Backport-PR-URL: #37100
PR-URL: #36178
Fixes: #35730
Fixes: #36151
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2021
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>

Backport-PR-URL: #37100
PR-URL: #36178
Fixes: #35730
Fixes: #36151
Refs: #35754
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
9 participants