lib: make sure console is writable#20185
Conversation
|
@kfarnung this is because of a change in node-chakra right? |
addaleax
left a comment
There was a problem hiding this comment.
Would be cool to have a test for this, if it's possible?
cjihrig
left a comment
There was a problem hiding this comment.
LGTM. +1 to adding a test.
|
@devsnek That's basically correct. At some point V8 moved their console object initialization out of the inspector code so they always initialize the @addaleax @cjihrig There are already existing tests for the behavior, but they fail in node-chakracore if we build without inspector (which is currently true for our WoA builds). See #17708 which added those tests. I'll add ref to that PR to this commit as well. |
|
Rebased and kicked off a new CI: https://ci.nodejs.org/job/node-test-pull-request/14421/ |
|
The CI failures seem unrelated (there should be no functional change here): https://ci.nodejs.org/job/node-test-commit-plinux/17100/nodes=ppcle-ubuntu1404/consoleText https://ci.nodejs.org/job/node-test-commit-osx/18000/nodes=osx1010/consoleText |
|
Looks like the macOS leg has some leftover processes: Seems to be another instance of #20194. |
The code currently assumes that `console` is already writable, but that's only if it was previously defined as writable. If it hasn't already been defined then the default value is false. Refs: #17708 PR-URL: #20185 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 982adb5 |
The code currently assumes that `console` is already writable, but that's only if it was previously defined as writable. If it hasn't already been defined then the default value is false. Refs: #17708 PR-URL: #20185 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The code currently assumes that
consoleis already writable, butthat's only if it was previously defined as writable. If it hasn't
already been defined then the default value is false.
Refs: #17708
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes