Revert "url: fix treatment of some values as non-empty"

This reverts commit 66877216bd833face753d9a5d573ad477895d880.

It was agreed that this change contained too much potential ecosystem
breakage, particularly around the inability to `delete` properties off a
`Url` object. It may be re-introduced for a later release, along with
better work on ecosystem compatibility.

PR-URL: #1602
Reviewed-By: Mikeal Rogers <mikeal.rogers@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Isaac Z. Schlueter <i@izs.me>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

diff --git a/lib/url.js b/lib/url.js
index df04ebc..52c3903 100644 (file)
--- a/lib/url.js
+++ b/lib/url.js
@@ -879,7 +879,7 @@ Object.defineProperty(Url.prototype, 'port', {
     return null;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._port = -1;
       if (this._host)
         this._host = null;
@@ -941,7 +941,7 @@ Object.defineProperty(Url.prototype, 'path', {
     return (p == null && s) ? ('/' + s) : null;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._pathname = this._search = null;
       return;
     }
@@ -973,7 +973,7 @@ Object.defineProperty(Url.prototype, 'protocol', {
     return proto;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._protocol = null;
     } else {
       var proto = '' + v;
@@ -1001,7 +1001,7 @@ Object.defineProperty(Url.prototype, 'href', {
     var parsesQueryStrings = this._parsesQueryStrings;
     // Reset properties.
     Url.call(this);
-    if (!isConsideredEmpty(v))
+    if (v !== null && v !== '')
       this.parse('' + v, parsesQueryStrings, false);
   },
   enumerable: true,
@@ -1013,7 +1013,7 @@ Object.defineProperty(Url.prototype, 'auth', {
     return this._auth;
   },
   set: function(v) {
-    this._auth = isConsideredEmpty(v) ? null : '' + v;
+    this._auth = v === null ? null : '' + v;
     this._href = '';
   },
   enumerable: true,
@@ -1026,7 +1026,7 @@ Object.defineProperty(Url.prototype, 'host', {
     return this._host;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._port = -1;
       this._hostname = this._host = null;
     } else {
@@ -1053,7 +1053,7 @@ Object.defineProperty(Url.prototype, 'hostname', {
     return this._hostname;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._hostname = null;
 
       if (this._hasValidPort())
@@ -1080,7 +1080,7 @@ Object.defineProperty(Url.prototype, 'hash', {
     return this._hash;
   },
   set: function(v) {
-    if (isConsideredEmpty(v) || v === '#') {
+    if (v === null) {
       this._hash = null;
     } else {
       var hash = '' + v;
@@ -1100,7 +1100,7 @@ Object.defineProperty(Url.prototype, 'search', {
     return this._search;
   },
   set: function(v) {
-    if (isConsideredEmpty(v) || v === '?') {
+    if (v === null) {
       this._search = this._query = null;
     } else {
       var search = escapeSearch('' + v);
@@ -1125,7 +1125,7 @@ Object.defineProperty(Url.prototype, 'pathname', {
     return this._pathname;
   },
   set: function(v) {
-    if (isConsideredEmpty(v)) {
+    if (v === null) {
       this._pathname = null;
     } else {
       var pathname = escapePathName('' + v).replace(/\\/g, '/');
@@ -1144,10 +1144,6 @@ Object.defineProperty(Url.prototype, 'pathname', {
   configurable: true
 });
 
-function isConsideredEmpty(value) {
-  return value === null || value === undefined || value === '';
-}
-
 // Search `char1` (integer code for a character) in `string`
 // starting from `fromIndex` and ending at `string.length - 1`
 // or when a stop character is found.

diff --git a/test/parallel/test-url-accessors.js b/test/parallel/test-url-accessors.js
index 3c39fd6..387c149 100644 (file)
--- a/test/parallel/test-url-accessors.js
+++ b/test/parallel/test-url-accessors.js
@@ -43,10 +43,10 @@ const accessorTests = [{
 }, {
   // Setting href to non-null non-string coerces to string
   url: 'google',
-  set: {href: 0},
+  set: {href: undefined},
   test: {
-    path: '0',
-    href: '0'
+    path: 'undefined',
+    href: 'undefined'
   }
 }, {
   // Setting port is reflected in host
@@ -180,8 +180,8 @@ const accessorTests = [{
   url: 'http://www.google.com',
   set: {search: ''},
   test: {
-    search: null,
-    path: '/'
+    search: '?',
+    path: '/?'
   }
 }, {
 
@@ -203,11 +203,10 @@ const accessorTests = [{
 }, {
 
   // Empty hash is ok
-  url: 'http://www.google.com#hash',
+  url: 'http://www.google.com',
   set: {hash: ''},
   test: {
-    hash: null,
-    href: 'http://www.google.com/'
+    hash: '#'
   }
 }, {
 
@@ -253,8 +252,7 @@ const accessorTests = [{
   url: 'http://www.google.com',
   set: {pathname: ''},
   test: {
-    pathname: null,
-    href: 'http://www.google.com'
+    pathname: '/'
   }
 }, {
   // Null path is ok
@@ -292,12 +290,11 @@ const accessorTests = [{
     protocol: null
   }
 }, {
-  // Empty protocol is ok
+  // Empty protocol is invalid
   url: 'http://www.google.com/path',
   set: {protocol: ''},
   test: {
-    protocol: null,
-    href: '//www.google.com/path'
+    protocol: 'http:'
   }
 }, {
   // Set query to an object
@@ -330,9 +327,9 @@ const accessorTests = [{
   url: 'http://www.google.com/path?key=value',
   set: {path: '?key2=value2'},
   test: {
-    pathname: null,
+    pathname: '/',
     search: '?key2=value2',
-    href: 'http://www.google.com?key2=value2'
+    href: 'http://www.google.com/?key2=value2'
   }
 }, {
   // path is reflected in search and pathname 3
@@ -352,38 +349,6 @@ const accessorTests = [{
     search: null,
     href: 'http://www.google.com'
   }
-}, {
-  // setting hash to '' removes any hash
-  url: 'http://www.google.com/#hash',
-  set: {hash: ''},
-  test: {
-    hash: null,
-    href: 'http://www.google.com/'
-  }
-}, {
-  // setting hash to '#' removes any hash
-  url: 'http://www.google.com/#hash',
-  set: {hash: '#'},
-  test: {
-    hash: null,
-    href: 'http://www.google.com/'
-  }
-}, {
-  // setting search to '' removes any search
-  url: 'http://www.google.com/?search',
-  set: {search: ''},
-  test: {
-    search: null,
-    href: 'http://www.google.com/'
-  }
-}, {
-  // setting search to '?' removes any search
-  url: 'http://www.google.com/?search',
-  set: {search: '?'},
-  test: {
-    search: null,
-    href: 'http://www.google.com/'
-  }
 }
 
 ];