Skip to content

console: add Symbol.toStringTag property#35399

Closed
Leko wants to merge 1 commit into
nodejs:masterfrom
Leko:wpt-console-SymbolToStringTag
Closed

console: add Symbol.toStringTag property#35399
Leko wants to merge 1 commit into
nodejs:masterfrom
Leko:wpt-console-SymbolToStringTag

Conversation

@Leko
Copy link
Copy Markdown
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
@Leko
console: add Symbol.toStringTag property
a5dad17 
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
Copy Markdown
Collaborator

nodejs-github-bot commented Sep 29, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/33282/
@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
Copy Markdown
Collaborator

nodejs-github-bot commented Oct 1, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/33313/
@Leko
Copy link
Copy Markdown
Contributor Author

Leko commented Oct 1, 2020

Landed in 4a6005c
Leko added a commit that referenced this pull request Oct 1, 2020
@Leko
console: add Symbol.toStringTag property
4a6005c 
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
@Leko @danielleadams
console: add Symbol.toStringTag property
05e08bd 
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
Merged
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
@Leko @MylesBorins
console: add Symbol.toStringTag property
d54c67a 
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
Merged
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
@Leko @MylesBorins
console: add Symbol.toStringTag property
2e766a6 
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
@facebook-github-bot
Fix worker-test and fs.createWriteStream spec incompliance
7667838 
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
@Leko @joesepi
console: add Symbol.toStringTag property
ffcc985 
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>
@targos targos mentioned this pull request Jan 23, 2021
Closed
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.