chore: address code review

nodejs · Jan 11, 2020 · 3ea89d2 · 3ea89d2
1 parent bc7c0c0
commit 3ea89d2
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 16 deletions.
diff --git a/doc/api/modules.md b/doc/api/modules.md
@@ -1034,8 +1034,9 @@ import('fs').then((esmFS) => {
 ```
 
 ## Source Map V3 Support
-
-<!--introduced_in=REPLACEME-->
+<!-- YAML
+added: REPLACEME
+-->
 
 > Stability: 1 - Experimental
 

diff --git a/lib/internal/source_map/source_map.js b/lib/internal/source_map/source_map.js
@@ -66,6 +66,14 @@
 
 'use strict';
 
+const {
+  Array
+} = primordials;
+
+const {
+  ERR_INVALID_ARG_TYPE
+} = require('internal/errors').codes;
+
 let base64Map;
 
 const VLQ_BASE_SHIFT = 5;
@@ -112,7 +120,7 @@ class StringCharIterator {
  * @param {SourceMapV3} payload
  */
 class SourceMap {
-  #payload = null;
+  #payload;
   #reverseMappingsBySourceURL = [];
   #mappings = [];
   #sources = {};
@@ -130,15 +138,15 @@ class SourceMap {
       for (let i = 0; i < base64Digits.length; ++i)
         base64Map[base64Digits[i]] = i;
     }
-    this.#payload = payload;
+    this.#payload = cloneSourceMapV3(payload);
     this.#parseMappingPayload();
   }
 
   /**
    * @return {Object} raw source map v3 payload.
    */
   get payload() {
-    return this.#payload;
+    return cloneSourceMapV3(this.#payload);
   }
 
   /**
@@ -169,13 +177,6 @@ class SourceMap {
   findEntry(lineNumber, columnNumber) {
     let first = 0;
     let count = this.#mappings.length;
-    const nullEntry = {
-      generatedLine: null,
-      generatedColumn: null,
-      originalSource: null,
-      originalLine: null,
-      originalColumn: null
-    };
     while (count > 1) {
       const step = count >> 1;
       const middle = first + step;
@@ -191,9 +192,9 @@ class SourceMap {
     const entry = this.#mappings[first];
     if (!first && entry && (lineNumber < entry[0] ||
         (lineNumber === entry[0] && columnNumber < entry[1]))) {
-      return nullEntry;
+      return {};
     } else if (!entry) {
-      return nullEntry;
+      return {};
     } else {
       return {
         generatedLine: entry[0],
@@ -306,6 +307,23 @@ function decodeVLQ(stringCharIterator) {
   return negative ? -result : result;
 }
 
+/**
+ * @param {SourceMapV3} payload
+ * @return {SourceMapV3}
+ */
+function cloneSourceMapV3(payload) {
+  if (typeof payload !== 'object') {
+    throw new ERR_INVALID_ARG_TYPE('payload', ['Object'], payload);
+  }
+  payload = { ...payload };
+  for (const key in payload) {
+    if (payload.hasOwnProperty(key) && Array.isArray(payload[key])) {
+      payload[key] = payload[key].slice(0);
+    }
+  }
+  return payload;
+}
+
 module.exports = {
   SourceMap
 };
diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js
@@ -244,7 +244,7 @@ function findSourceMap(uri, error) {
   if (sourceMap && sourceMap.data) {
     return new SourceMap(sourceMap.data);
   } else {
-    return null;
+    return undefined;
   }
 }
 

diff --git a/test/parallel/test-source-map-api.js b/test/parallel/test-source-map-api.js
@@ -76,5 +76,9 @@ const { readFileSync } = require('fs');
   assert.strictEqual(originalLine, 2);
   assert.strictEqual(originalColumn, 4);
   assert(originalSource.endsWith('disk.js'));
-  assert.strictEqual(payload, sourceMap.payload);
+  // The stored payload should be a clone:
+  assert.strictEqual(payload.mappings, sourceMap.payload.mappings);
+  assert.notStrictEqual(payload, sourceMap.payload);
+  assert.strictEqual(payload.sources[0], sourceMap.payload.sources[0]);
+  assert.notStrictEqual(payload.sources, sourceMap.payload.sources);
 }