Skip to content

util: fix TypeError when console.log a static property name in class - #42790

Closed
cola119 wants to merge 2 commits into
nodejs:mainfrom
cola119:issue/42773
Closed

util: fix TypeError when console.log a static property name in class#42790
cola119 wants to merge 2 commits into
nodejs:mainfrom
cola119:issue/42773

Conversation

@cola119

@cola119 cola119 commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Fixed #42773

Constructor name from getConstructorName will be converted to string by template literal (${constructor}) in a subsequent process.

In this case, getConstructorName need to convert symbol to string by Symbol.prototype.toString beforehand since ${Symbol()} throws TypeError (spec).

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Apr 20, 2022
@meixg meixg added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 20, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Apr 20, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

ghost commented Apr 20, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43579/

This was referenced Apr 21, 2022
Open
Open
BridgeAR
BridgeAR reviewed Apr 22, 2022
View reviewed changes

ghost left a comment •
edited by BridgeAR
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is definitely the right spot where to fix it!

Ti improve it further, we could just use String(). That way other types are also handled properly.

Comment thread lib/internal/util/inspect.js Outdated
Comment on lines +582 to +583
const name = descriptor.value.name;
return typeof name === 'symbol' ? SymbolPrototypeToString(name) : name;

ghost Apr 22, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const name = descriptor.value.name;
return typeof name === 'symbol' ? SymbolPrototypeToString(name) : name;
return String(descriptor.value.name);

ghost Apr 23, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you. That makes sense!

@github-actions github-actions Bot mentioned this pull request Apr 23, 2022
Open
21 tasks
@cola119
cola119 requested a review from BridgeAR April 23, 2022 03:52
This was referenced Apr 24, 2022
Open
Open
@watilde watilde added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label May 19, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label May 19, 2022
@nodejs-github-bot

ghost commented May 19, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44079/

This was referenced May 20, 2022
Open
Open
Open
Open
Open
Open
Open
@cola119

ghost commented Jun 5, 2022

Copy link
Copy Markdown
Member Author

@jasnell @BridgeAR Could you please re-trigger the test and merge? Thank you in advance!

@BridgeAR BridgeAR added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Jun 11, 2022
@panva panva added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Jun 16, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jun 16, 2022
@nodejs-github-bot

ghost commented Jun 16, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44629/

@github-actions github-actions Bot mentioned this pull request Jun 17, 2022
Open
31 tasks
@nodejs-github-bot

ghost commented Jun 17, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44636/

@panva panva closed this Jun 17, 2022
@panva

ghost commented Jun 17, 2022

Copy link
Copy Markdown
Member

Landed in 027c288

This was referenced Jun 18, 2022
Open
Open
@targos targos mentioned this pull request Jul 12, 2022
Merged
@targos targos mentioned this pull request Aug 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@BridgeAR BridgeAR BridgeAR approved these changes

Assignees

No one assigned

Labels

commit-queue-squash PRs the Commit Queue should land as one squashed commit. needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeError when console.log a static property name in class

7 participants

@cola119 @nodejs-github-bot @panva @jasnell @BridgeAR @watilde @meixg