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

make use of util.inspect.custom #8738

Merged
merged 1 commit into from
Feb 6, 2019
Merged

make use of util.inspect.custom #8738

merged 1 commit into from
Feb 6, 2019

Conversation

hiroppy
Copy link
Member

@hiroppy hiroppy commented Feb 6, 2019

What kind of change does this PR introduce?

Currently, WebpackError.unittest.js test has failed.

Did you add tests for your changes?

no

Does this PR introduce a breaking change?

no

What needs to be documented once your changes are merged?

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@hiroppy
Copy link
Member Author

hiroppy commented Feb 6, 2019

This test gets "{ CustomError: CustomMessage" when using Node v11.4.0.
But when Node is v10.15.1, it will get "CustomError: CustomMessage".

@sokra Could you tell me the version of Node in this log? #8738 (comment)

@sokra
Copy link
Member

sokra commented Feb 6, 2019

node 10.

starting with node 11 util.inspect will no longer use the inspect property of the object.

In lib/WebpackError util.inspect.custom need to be used instead to make it consistent for node >10 too

See here: https://nodejs.org/docs/latest/api/util.html#util_util_inspect_custom

@hiroppy
Copy link
Member Author

hiroppy commented Feb 6, 2019

@sokra Thank you for the information! Updated the code. PTAL 🙇

@hiroppy hiroppy changed the title fix WebpackError.unittest make use of nodejs.util.inspect.custom Feb 6, 2019
Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

Seem like the CI didn't catch that, because unittest only run on node.js 10, but the node.js documentation states that Symbol.for('nodejs.util.inspect.custom') was added with node 10.12.0, while util.inspect.custom was added in node 6.

https://nodejs.org/docs/latest/api/util.html#util_util_inspect_custom in History

v10.12.0 | This is now defined as a shared symbol.

@@ -4,6 +4,8 @@
*/
"use strict";

const inspect = Symbol.for("nodejs.util.inspect.custom");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const inspect = Symbol.for("nodejs.util.inspect.custom");
const inspect = require("util").inspect.custom;

@webpack-bot
Copy link
Contributor

@hiroppy Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@hiroppy
Copy link
Member Author

hiroppy commented Feb 6, 2019

Ah, I see. Updated:)

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@sokra sokra merged commit 109db05 into webpack:master Feb 6, 2019
@sokra
Copy link
Member

sokra commented Feb 6, 2019

Thanks

@sokra sokra changed the title make use of nodejs.util.inspect.custom make use of util.inspect.custom Feb 6, 2019
@hiroppy hiroppy deleted the feature/modify-WebpackError.unittest branch February 6, 2019 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants