fixup! lib: add options to the heap snapshot APIs

joyeecheung · joyeecheung · commit cbe5987f9d00 · 2022-10-31T15:41:17.000+01:00
diff --git a/doc/api/v8.md b/doc/api/v8.md
@@ -67,16 +67,17 @@ following properties:
 added: v11.13.0
 changes:
   - version: REPLACEME
-    pr-url: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/44989
     description: Support options to configure the heap snapshot.
 -->
 
 * `options` {Object}
   * `exposeInternals` {boolean} If true, expose internals in the heap snapshot.
+    **Default:** `false`.
   * `exposeNumericValues` {boolean} If true, expose numeric values in
-    artificial fields.
+    artificial fields. **Default:** `false`.
 
-* Returns: {stream.Readable} A Readable Stream containing the V8 heap snapshot
+* Returns: {stream.Readable} A Readable containing the V8 heap snapshot.
 
 Generates a snapshot of the current V8 heap and returns a Readable
 Stream that may be used to read the JSON serialized representation.
@@ -310,7 +311,7 @@ changes:
     pr-url: https://github.com/nodejs/node/pull/42577
     description: Make the returned error codes consistent across all platforms.
   - version: REPLACEME
-    pr-url: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/44989
     description: Support options to configure the heap snapshot.
 -->
 
@@ -320,7 +321,11 @@ changes:
   generated, where `{pid}` will be the PID of the Node.js process,
   `{thread_id}` will be `0` when `writeHeapSnapshot()` is called from
   the main Node.js thread or the id of a worker thread.
-* `options` {Object} See [`v8.getHeapSnapshot()`][] for more details.
+* `options` {Object}
+  * `exposeInternals` {boolean} If true, expose internals in the heap snapshot.
+    **Default:** `false`.
+  * `exposeNumericValues` {boolean} If true, expose numeric values in
+    artificial fields. **Default:** `false`.
 * Returns: {string} The filename where the snapshot was saved.
 
 Generates a snapshot of the current V8 heap and writes it to a JSON
@@ -1075,7 +1080,6 @@ Returns true if the Node.js instance is run to build a snapshot.
 [`serializer.transferArrayBuffer()`]: #serializertransferarraybufferid-arraybuffer
 [`serializer.writeRawBytes()`]: #serializerwriterawbytesbuffer
 [`settled` callback]: #settledpromise
-[`v8.getHeapSnapshot()`]: #v8getheapsnapshot
 [`v8.stopCoverage()`]: #v8stopcoverage
 [`v8.takeCoverage()`]: #v8takecoverage
 [`vm.Script`]: vm.md#new-vmscriptcode-options
diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md
@@ -1075,11 +1075,15 @@ added:
  - v12.17.0
 changes:
   - version: REPLACEME
-    pr-url: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/44989
     description: Support options to configure the heap snapshot.
 -->
 
-* `options` {Object} See [`v8.getHeapSnapshot()`][] for more details.
+* `options` {Object}
+  * `exposeInternals` {boolean} If true, expose internals in the heap snapshot.
+    **Default:** `false`.
+  * `exposeNumericValues` {boolean} If true, expose numeric values in
+    artificial fields. **Default:** `false`.
 * Returns: {Promise} A promise for a Readable Stream containing
   a V8 heap snapshot
 
diff --git a/lib/internal/heap_utils.js b/lib/internal/heap_utils.js
@@ -10,25 +10,19 @@ const {
 const { owner_symbol } = require('internal/async_hooks').symbols;
 const { Readable } = require('stream');
 const { validateObject, validateBoolean } = require('internal/validators');
+const { kEmptyObject } = require('internal/util');
 
 const kHandle = Symbol('kHandle');
 
-function getHeapSnapshotOptions(options = {
-  exposeInternals: false,
-  exposeNumericValues: false
-}) {
+function getHeapSnapshotOptions(options = kEmptyObject) {
   validateObject(options, 'options');
-  if (options.exposeInternals === undefined) {
-    options.exposeInternals = false;
-  }
-  if (options.exposeNumericValues === undefined) {
-    options.exposeNumericValues = false;
-  }
+  options.exposeInternals ??= false;
+  options.exposeNumbericValues ??= false;
   validateBoolean(options.exposeInternals, 'options.exposeInternals');
   validateBoolean(options.exposeNumericValues, 'options.exposeNumericValues');
   return new Uint8Array([
     +options.exposeInternals,
-    +options.exposeNumericValues
+    +options.exposeNumericValues,
   ]);
 }
 
diff --git a/lib/internal/worker.js b/lib/internal/worker.js
@@ -422,7 +422,7 @@ class Worker extends EventEmitter {
       getHeapSnapshotOptions
     } = require('internal/heap_utils');
     const optionsArray = getHeapSnapshotOptions(options);
-    const heapSnapshotTaker = this[kHandle] && this[kHandle].takeHeapSnapshot(optionsArray);
+    const heapSnapshotTaker = this[kHandle]?.takeHeapSnapshot(optionsArray);
     return new Promise((resolve, reject) => {
       if (!heapSnapshotTaker) return reject(new ERR_WORKER_NOT_RUNNING());
       heapSnapshotTaker.ondone = (handle) => {
diff --git a/test/common/heap.js b/test/common/heap.js
@@ -239,7 +239,7 @@ function getHeapSnapshotOptionTests() {
       snapshot.validateSnapshot('Klass', expected, { loose: true });
     },
     cases,
-  }
+  };
 }
 
 module.exports = {

Original file line number	Diff line number	Diff line change
`@@ -239,7 +239,7 @@ function getHeapSnapshotOptionTests() {`
`239`	`239`	`snapshot.validateSnapshot('Klass', expected, { loose: true });`
`240`	`240`	`},`
`241`	`241`	`cases,`
`242`		`- }`
	`242`	`+ };`
`243`	`243`	`}`
`244`	`244`
`245`	`245`	`module.exports = {`