url: update sort() behavior with no params

PR-URL: #14185 Refs: whatwg/url#336 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Aug 1, 2017 · 3225942 · 3225942
1 parent cfed48e
commit 3225942
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 8 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1076,12 +1076,11 @@ defineIDLClass(URLSearchParams.prototype, 'URLSearchParams', {
   sort() {
     const a = this[searchParams];
     const len = a.length;
-    if (len <= 2) {
-      return;
-    }
 
-    // arbitrary number found through testing
-    if (len < 100) {
+    if (len <= 2) {
+      // Nothing needs to be done.
+    } else if (len < 100) {
+      // 100 is found through testing.
       // Simple stable in-place insertion sort
       // Derived from v8/src/js/array.js
       for (var i = 2; i < len; i += 2) {

diff --git a/test/parallel/test-whatwg-url-searchparams-delete.js b/test/parallel/test-whatwg-url-searchparams-delete.js
@@ -8,7 +8,7 @@ const { test, assert_equals, assert_true, assert_false } =
 
 /* The following tests are copied from WPT. Modifications to them should be
    upstreamed first. Refs:
-   https://github.com/w3c/web-platform-tests/blob/8791bed/url/urlsearchparams-delete.html
+   https://github.com/w3c/web-platform-tests/blob/70a0898763/url/urlsearchparams-delete.html
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
 /* eslint-disable */
@@ -42,6 +42,21 @@ test(function() {
     params.delete('first');
     assert_false(params.has('first'), 'Search params object has no "first" name');
 }, 'Deleting appended multiple');
+
+test(function() {
+    var url = new URL('http://example.com/?param1&param2');
+    url.searchParams.delete('param1');
+    url.searchParams.delete('param2');
+    assert_equals(url.href, 'http://example.com/', 'url.href does not have ?');
+    assert_equals(url.search, '', 'url.search does not have ?');
+}, 'Deleting all params removes ? from URL');
+
+test(function() {
+    var url = new URL('http://example.com/?');
+    url.searchParams.delete('param1');
+    assert_equals(url.href, 'http://example.com/', 'url.href does not have ?');
+    assert_equals(url.search, '', 'url.search does not have ?');
+}, 'Removing non-existent param removes ? from URL');
 /* eslint-enable */
 
 // Tests below are not from WPT.

diff --git a/test/parallel/test-whatwg-url-searchparams-sort.js b/test/parallel/test-whatwg-url-searchparams-sort.js
@@ -2,11 +2,11 @@
 
 require('../common');
 const { URL, URLSearchParams } = require('url');
-const { test, assert_array_equals } = require('../common/wpt');
+const { test, assert_equals, assert_array_equals } = require('../common/wpt');
 
 /* The following tests are copied from WPT. Modifications to them should be
    upstreamed first. Refs:
-   https://github.com/w3c/web-platform-tests/blob/5903e00e77e85f8bcb21c73d1d7819fcd04763bd/url/urlsearchparams-sort.html
+   https://github.com/w3c/web-platform-tests/blob/70a0898763/url/urlsearchparams-sort.html
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
 /* eslint-disable */
@@ -53,6 +53,13 @@ const { test, assert_array_equals } = require('../common/wpt');
     }
   }, "URL parse and sort: " + val.input)
 })
+
+test(function() {
+  const url = new URL("http://example.com/?")
+  url.searchParams.sort()
+  assert_equals(url.href, "http://example.com/")
+  assert_equals(url.search, "")
+}, "Sorting non-existent params removes ? from URL")
 /* eslint-enable */
 
 // Tests below are not from WPT.