url: make URLSearchParams/Iterator match spec

PR-URL: https://github.com/nodejs/node/pull/11057
Fixes: https://github.com/nodejs/node/issues/10799
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

lib/internal/url.js

@@ -647,6 +647,35 @@ function getObjectFromParams(array) {
   return obj;
 }
 
+// Mainly to mitigate func-name-matching ESLint rule
+function defineIDLClass(proto, classStr, obj) {
+  // https://heycam.github.io/webidl/#dfn-class-string
+  Object.defineProperty(proto, Symbol.toStringTag, {
+    writable: false,
+    enumerable: false,
+    configurable: true,
+    value: classStr
+  });
+
+  // https://heycam.github.io/webidl/#es-operations
+  for (const key of Object.keys(obj)) {
+    Object.defineProperty(proto, key, {
+      writable: true,
+      enumerable: true,
+      configurable: true,
+      value: obj[key]
+    });
+  }
+  for (const key of Object.getOwnPropertySymbols(obj)) {
+    Object.defineProperty(proto, key, {
+      writable: true,
+      enumerable: false,
+      configurable: true,
+      value: obj[key]
+    });
+  }
+}
+
 class URLSearchParams {
   constructor(init = '') {
     if (init instanceof URLSearchParams) {
@@ -662,11 +691,39 @@ class URLSearchParams {
     this[context] = null;
   }
 
-  get [Symbol.toStringTag]() {
+  [util.inspect.custom](recurseTimes, ctx) {
-    return this instanceof URLSearchParams ?
+    if (!this || !(this instanceof URLSearchParams)) {
-      'URLSearchParams' : 'URLSearchParamsPrototype';
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+
+    const separator = ', ';
+    const innerOpts = Object.assign({}, ctx);
+    if (recurseTimes !== null) {
+      innerOpts.depth = recurseTimes - 1;
+    }
+    const innerInspect = (v) => util.inspect(v, innerOpts);
+
+    const list = this[searchParams];
+    const output = [];
+    for (var i = 0; i < list.length; i += 2)
+      output.push(`${innerInspect(list[i])} => ${innerInspect(list[i + 1])}`);
+
+    const colorRe = /\u001b\[\d\d?m/g;
+    const length = output.reduce(
+      (prev, cur) => prev + cur.replace(colorRe, '').length + separator.length,
+      -separator.length
+    );
+    if (length > ctx.breakLength) {
+      return `${this.constructor.name} {\n  ${output.join(',\n  ')} }`;
+    } else if (output.length) {
+      return `${this.constructor.name} { ${output.join(separator)} }`;
+    } else {
+      return `${this.constructor.name} {}`;
+    }
   }
+}
 
+defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
   append(name, value) {
     if (!this || !(this instanceof URLSearchParams)) {
       throw new TypeError('Value of `this` is not a URLSearchParams');
@@ -679,7 +736,7 @@ class URLSearchParams {
     value = String(value);
     this[searchParams].push(name, value);
     update(this[context], this);
-  }
+  },
 
   delete(name) {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -700,47 +757,7 @@ class URLSearchParams {
       }
     }
     update(this[context], this);
-  }
+  },
-
-  set(name, value) {
-    if (!this || !(this instanceof URLSearchParams)) {
-      throw new TypeError('Value of `this` is not a URLSearchParams');
-    }
-    if (arguments.length < 2) {
-      throw new TypeError('"name" and "value" arguments must be specified');
-    }
-
-    const list = this[searchParams];
-    name = String(name);
-    value = String(value);
-
-    // If there are any name-value pairs whose name is `name`, in `list`, set
-    // the value of the first such name-value pair to `value` and remove the
-    // others.
-    var found = false;
-    for (var i = 0; i < list.length;) {
-      const cur = list[i];
-      if (cur === name) {
-        if (!found) {
-          list[i + 1] = value;
-          found = true;
-          i += 2;
-        } else {
-          list.splice(i, 2);
-        }
-      } else {
-        i += 2;
-      }
-    }
-
-    // Otherwise, append a new name-value pair whose name is `name` and value
-    // is `value`, to `list`.
-    if (!found) {
-      list.push(name, value);
-    }
-
-    update(this[context], this);
-  }
 
   get(name) {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -758,7 +775,7 @@ class URLSearchParams {
       }
     }
     return null;
-  }
+  },
 
   getAll(name) {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -777,7 +794,7 @@ class URLSearchParams {
       }
     }
     return values;
-  }
+  },
 
   has(name) {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -795,7 +812,47 @@ class URLSearchParams {
       }
     }
     return false;
-  }
+  },
+
+  set(name, value) {
+    if (!this || !(this instanceof URLSearchParams)) {
+      throw new TypeError('Value of `this` is not a URLSearchParams');
+    }
+    if (arguments.length < 2) {
+      throw new TypeError('"name" and "value" arguments must be specified');
+    }
+
+    const list = this[searchParams];
+    name = String(name);
+    value = String(value);
+
+    // If there are any name-value pairs whose name is `name`, in `list`, set
+    // the value of the first such name-value pair to `value` and remove the
+    // others.
+    var found = false;
+    for (var i = 0; i < list.length;) {
+      const cur = list[i];
+      if (cur === name) {
+        if (!found) {
+          list[i + 1] = value;
+          found = true;
+          i += 2;
+        } else {
+          list.splice(i, 2);
+        }
+      } else {
+        i += 2;
+      }
+    }
+
+    // Otherwise, append a new name-value pair whose name is `name` and value
+    // is `value`, to `list`.
+    if (!found) {
+      list.push(name, value);
+    }
+
+    update(this[context], this);
+  },
 
   // https://heycam.github.io/webidl/#es-iterators
   // Define entries here rather than [Symbol.iterator] as the function name
@@ -806,7 +863,7 @@ class URLSearchParams {
     }
 
     return createSearchParamsIterator(this, 'key+value');
-  }
+  },
 
   forEach(callback, thisArg = undefined) {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -827,7 +884,7 @@ class URLSearchParams {
       list = this[searchParams];
       i += 2;
     }
-  }
+  },
 
   // https://heycam.github.io/webidl/#es-iterable
   keys() {
@@ -836,7 +893,7 @@ class URLSearchParams {
     }
 
     return createSearchParamsIterator(this, 'key');
-  }
+  },
 
   values() {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -844,8 +901,9 @@ class URLSearchParams {
     }
 
     return createSearchParamsIterator(this, 'value');
-  }
+  },
 
+  // https://heycam.github.io/webidl/#es-stringifier
   // https://url.spec.whatwg.org/#urlsearchparams-stringification-behavior
   toString() {
     if (!this || !(this instanceof URLSearchParams)) {
@@ -854,37 +912,14 @@ class URLSearchParams {
 
     return querystring.stringify(getObjectFromParams(this[searchParams]));
   }
-}
+});
-// https://heycam.github.io/webidl/#es-iterable-entries
-URLSearchParams.prototype[Symbol.iterator] = URLSearchParams.prototype.entries;
-
-URLSearchParams.prototype[util.inspect.custom] =
-  function inspect(recurseTimes, ctx) {
-    const separator = ', ';
-    const innerOpts = Object.assign({}, ctx);
-    if (recurseTimes !== null) {
-      innerOpts.depth = recurseTimes - 1;
-    }
-    const innerInspect = (v) => util.inspect(v, innerOpts);
-
-    const list = this[searchParams];
-    const output = [];
-    for (var i = 0; i < list.length; i += 2)
-      output.push(`${innerInspect(list[i])} => ${innerInspect(list[i + 1])}`);
 
-    const colorRe = /\u001b\[\d\d?m/g;
+// https://heycam.github.io/webidl/#es-iterable-entries
-    const length = output.reduce(
+Object.defineProperty(URLSearchParams.prototype, Symbol.iterator, {
-      (prev, cur) => prev + cur.replace(colorRe, '').length + separator.length,
+  writable: true,
-      -separator.length
+  configurable: true,
-    );
+  value: URLSearchParams.prototype.entries
-    if (length > ctx.breakLength) {
+});
-      return `${this.constructor.name} {\n  ${output.join(',\n  ')} }`;
-    } else if (output.length) {
-      return `${this.constructor.name} { ${output.join(separator)} }`;
-    } else {
-      return `${this.constructor.name} {}`;
-    }
-  };
 
 // https://heycam.github.io/webidl/#dfn-default-iterator-object
 function createSearchParamsIterator(target, kind) {
@@ -898,7 +933,9 @@ function createSearchParamsIterator(target, kind) {
 }
 
 // https://heycam.github.io/webidl/#dfn-iterator-prototype-object
-const URLSearchParamsIteratorPrototype = Object.setPrototypeOf({
+const URLSearchParamsIteratorPrototype = Object.create(IteratorPrototype);
+
+defineIDLClass(URLSearchParamsIteratorPrototype, 'URLSearchParamsIterator', {
   next() {
     if (!this ||
         Object.getPrototypeOf(this) !== URLSearchParamsIteratorPrototype) {
@@ -937,7 +974,7 @@ const URLSearchParamsIteratorPrototype = Object.setPrototypeOf({
       done: false
     };
   },
-  [util.inspect.custom]: function inspect(recurseTimes, ctx) {
+  [util.inspect.custom](recurseTimes, ctx) {
     const innerOpts = Object.assign({}, ctx);
     if (recurseTimes !== null) {
       innerOpts.depth = recurseTimes - 1;
@@ -968,15 +1005,6 @@ const URLSearchParamsIteratorPrototype = Object.setPrototypeOf({
     }
     return `${this[Symbol.toStringTag]} {${outputStr} }`;
   }
-}, IteratorPrototype);
-
-// Unlike interface and its prototype object, both default iterator object and
-// iterator prototype object of an interface have the same class string.
-Object.defineProperty(URLSearchParamsIteratorPrototype, Symbol.toStringTag, {
-  value: 'URLSearchParamsIterator',
-  writable: false,
-  enumerable: false,
-  configurable: true
 });
 
 function originFor(url, base) {

test/parallel/test-whatwg-url-tostringtag.js

@@ -7,18 +7,24 @@ const toString = Object.prototype.toString;
 
 const url = new URL('http://example.org');
 const sp = url.searchParams;
+const spIterator = sp.entries();
 
 const test = [
-  [toString.call(url), 'URL'],
+  [url, 'URL'],
-  [toString.call(sp), 'URLSearchParams'],
+  [sp, 'URLSearchParams'],
-  [toString.call(Object.getPrototypeOf(sp)), 'URLSearchParamsPrototype'],
+  [spIterator, 'URLSearchParamsIterator'],
   // Web IDL spec says we have to return 'URLPrototype', but it is too
   // expensive to implement; therefore, use Chrome's behavior for now, until
   // spec is changed.
-  [toString.call(Object.getPrototypeOf(url)), 'URL']
+  [Object.getPrototypeOf(url), 'URL'],
+  [Object.getPrototypeOf(sp), 'URLSearchParams'],
+  [Object.getPrototypeOf(spIterator), 'URLSearchParamsIterator'],
 ];
 
-test.forEach((row) => {
+test.forEach(([obj, expected]) => {
-  assert.strictEqual(row[0], `[object ${row[1]}]`,
+  assert.strictEqual(obj[Symbol.toStringTag], expected,
-                     `${row[0]} !== [object ${row[1]}]`);
+                     `${obj[Symbol.toStringTag]} !== ${expected}`);
+  const str = toString.call(obj);
+  assert.strictEqual(str, `[object ${expected}]`,
+                     `${str} !== [object ${expected}]`);
 });