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

console: add Symbol.toStringTag property #35399

Closed
wants to merge 1 commit into from
Closed

console: add Symbol.toStringTag property #35399

wants to merge 1 commit into from

Conversation

Leko
Copy link
Contributor

@Leko Leko commented Sep 28, 2020

Add Symbol.toStringTag property to the console object to follow the WPT spec changes.
Update the console WPT status and the repl test case.

Refs: web-platform-tests/wpt#24717

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

Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717
@nodejs-github-bot nodejs-github-bot added the console Issues and PRs related to the console subsystem. label Sep 28, 2020
@Leko Leko added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 29, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@Leko
Copy link
Contributor Author

Leko commented Oct 1, 2020

Landed in 4a6005c

Leko added a commit that referenced this pull request Oct 1, 2020
Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717

PR-URL: #35399
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Leko Leko closed this Oct 1, 2020
@Leko Leko deleted the wpt-console-SymbolToStringTag branch October 1, 2020 15:54
danielleadams pushed a commit that referenced this pull request Oct 6, 2020
Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717

PR-URL: #35399
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request Oct 6, 2020
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717

PR-URL: #35399
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2020
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717

PR-URL: #35399
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
facebook-github-bot pushed a commit to facebook/metro that referenced this pull request Dec 12, 2020
Summary:
The CircleCI build of metro is currently failing because `jest` fails to mock the `console` module. This is because the typeof console changed in the latest node version ([PR](nodejs/node#35399)). Meaning that the typeof `console` is `console` and Jest doesn't know how to mock a value of type `console`.

This diff manually mocks the `console` object to avoid this issue (verified with using Node 15 locally). However, the tests then started failing because the `close` event of streams was emitted more than once by our memory fs implementation.

The issue is that our `createWriteStream` implementetion does not specify the `emitClose: false` when creating the stream as required according to the [docs](https://nodejs.org/api/fs.html#fs_fs_createwritestream_path_options):

> By default, the stream will not emit a 'close' event after it has been destroyed. This is the opposite of the default for other Writable streams. Set the emitClose option to true to change this behavior.

I changed that in our `memory-fs` implementation and removed the manual emit of the `close` event. I then had to change the tests because the order of the events is `end`, `finish`, `close` and the tests asserted in the `end` event.

The tests then started passing on Node 15 but, surprise, they now failed on Node 12...

The reason is that the [`stream.Writeable`](https://nodejs.org/docs/latest-v13.x/api/stream.html#stream_constructor_new_stream_writable_options) option `autoDestroy` has changed with Node13 from default false to default true. Hardcoding `default: true` finally makes all tests passing on Node 12 - 15.

Reviewed By: cpojer

Differential Revision: D25495034

fbshipit-source-id: fdf871fa4dfbec079bc9ec1843fcff7facb3e0be
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
Add Symbol.toStringTag property to console object to follow WPT changes
Update WPT status of console and the repl test case

Refs: web-platform-tests/wpt#24717

PR-URL: nodejs#35399
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.