Skip to content

Commit 0f39ef4

Browse files
rvaggrvagg
committed
Revert "url: fix treatment of some values as non-empty"
This reverts commit 6687721. 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 <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Forrest L Norvell <[email protected]> Reviewed-By: Chris Dickinson <[email protected]> Reviewed-By: Isaac Z. Schlueter <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
1 parent 6687721 commit 0f39ef4

2 files changed

Lines changed: 22 additions & 61 deletions

File tree

‎lib/url.js‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ Object.defineProperty(Url.prototype, 'port', {
879879
return null;
880880
},
881881
set: function(v) {
882-
if (isConsideredEmpty(v)) {
882+
if (v === null) {
883883
this._port = -1;
884884
if (this._host)
885885
this._host = null;
@@ -941,7 +941,7 @@ Object.defineProperty(Url.prototype, 'path', {
941941
return (p == null && s) ? ('/' + s) : null;
942942
},
943943
set: function(v) {
944-
if (isConsideredEmpty(v)) {
944+
if (v === null) {
945945
this._pathname = this._search = null;
946946
return;
947947
}
@@ -973,7 +973,7 @@ Object.defineProperty(Url.prototype, 'protocol', {
973973
return proto;
974974
},
975975
set: function(v) {
976-
if (isConsideredEmpty(v)) {
976+
if (v === null) {
977977
this._protocol = null;
978978
} else {
979979
var proto = '' + v;
@@ -1001,7 +1001,7 @@ Object.defineProperty(Url.prototype, 'href', {
10011001
var parsesQueryStrings = this._parsesQueryStrings;
10021002
// Reset properties.
10031003
Url.call(this);
1004-
if (!isConsideredEmpty(v))
1004+
if (v !== null && v !== '')
10051005
this.parse('' + v, parsesQueryStrings, false);
10061006
},
10071007
enumerable: true,
@@ -1013,7 +1013,7 @@ Object.defineProperty(Url.prototype, 'auth', {
10131013
return this._auth;
10141014
},
10151015
set: function(v) {
1016-
this._auth = isConsideredEmpty(v) ? null : '' + v;
1016+
this._auth = v === null ? null : '' + v;
10171017
this._href = '';
10181018
},
10191019
enumerable: true,
@@ -1026,7 +1026,7 @@ Object.defineProperty(Url.prototype, 'host', {
10261026
return this._host;
10271027
},
10281028
set: function(v) {
1029-
if (isConsideredEmpty(v)) {
1029+
if (v === null) {
10301030
this._port = -1;
10311031
this._hostname = this._host = null;
10321032
} else {
@@ -1053,7 +1053,7 @@ Object.defineProperty(Url.prototype, 'hostname', {
10531053
return this._hostname;
10541054
},
10551055
set: function(v) {
1056-
if (isConsideredEmpty(v)) {
1056+
if (v === null) {
10571057
this._hostname = null;
10581058

10591059
if (this._hasValidPort())
@@ -1080,7 +1080,7 @@ Object.defineProperty(Url.prototype, 'hash', {
10801080
return this._hash;
10811081
},
10821082
set: function(v) {
1083-
if (isConsideredEmpty(v) || v === '#') {
1083+
if (v === null) {
10841084
this._hash = null;
10851085
} else {
10861086
var hash = '' + v;
@@ -1100,7 +1100,7 @@ Object.defineProperty(Url.prototype, 'search', {
11001100
return this._search;
11011101
},
11021102
set: function(v) {
1103-
if (isConsideredEmpty(v) || v === '?') {
1103+
if (v === null) {
11041104
this._search = this._query = null;
11051105
} else {
11061106
var search = escapeSearch('' + v);
@@ -1125,7 +1125,7 @@ Object.defineProperty(Url.prototype, 'pathname', {
11251125
return this._pathname;
11261126
},
11271127
set: function(v) {
1128-
if (isConsideredEmpty(v)) {
1128+
if (v === null) {
11291129
this._pathname = null;
11301130
} else {
11311131
var pathname = escapePathName('' + v).replace(/\\/g, '/');
@@ -1144,10 +1144,6 @@ Object.defineProperty(Url.prototype, 'pathname', {
11441144
configurable: true
11451145
});
11461146

1147-
function isConsideredEmpty(value) {
1148-
return value === null || value === undefined || value === '';
1149-
}
1150-
11511147
// Search `char1` (integer code for a character) in `string`
11521148
// starting from `fromIndex` and ending at `string.length - 1`
11531149
// or when a stop character is found.

‎test/parallel/test-url-accessors.js‎

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ const accessorTests = [{
4343
}, {
4444
// Setting href to non-null non-string coerces to string
4545
url: 'google',
46-
set: {href: 0},
46+
set: {href: undefined},
4747
test: {
48-
path: '0',
49-
href: '0'
48+
path: 'undefined',
49+
href: 'undefined'
5050
}
5151
}, {
5252
// Setting port is reflected in host
@@ -180,8 +180,8 @@ const accessorTests = [{
180180
url: 'http://www.google.com',
181181
set: {search: ''},
182182
test: {
183-
search: null,
184-
path: '/'
183+
search: '?',
184+
path: '/?'
185185
}
186186
}, {
187187

@@ -203,11 +203,10 @@ const accessorTests = [{
203203
}, {
204204

205205
// Empty hash is ok
206-
url: 'http://www.google.com#hash',
206+
url: 'http://www.google.com',
207207
set: {hash: ''},
208208
test: {
209-
hash: null,
210-
href: 'http://www.google.com/'
209+
hash: '#'
211210
}
212211
}, {
213212

@@ -253,8 +252,7 @@ const accessorTests = [{
253252
url: 'http://www.google.com',
254253
set: {pathname: ''},
255254
test: {
256-
pathname: null,
257-
href: 'http://www.google.com'
255+
pathname: '/'
258256
}
259257
}, {
260258
// Null path is ok
@@ -292,12 +290,11 @@ const accessorTests = [{
292290
protocol: null
293291
}
294292
}, {
295-
// Empty protocol is ok
293+
// Empty protocol is invalid
296294
url: 'http://www.google.com/path',
297295
set: {protocol: ''},
298296
test: {
299-
protocol: null,
300-
href: '//www.google.com/path'
297+
protocol: 'http:'
301298
}
302299
}, {
303300
// Set query to an object
@@ -330,9 +327,9 @@ const accessorTests = [{
330327
url: 'http://www.google.com/path?key=value',
331328
set: {path: '?key2=value2'},
332329
test: {
333-
pathname: null,
330+
pathname: '/',
334331
search: '?key2=value2',
335-
href: 'http://www.google.com?key2=value2'
332+
href: 'http://www.google.com/?key2=value2'
336333
}
337334
}, {
338335
// path is reflected in search and pathname 3
@@ -352,38 +349,6 @@ const accessorTests = [{
352349
search: null,
353350
href: 'http://www.google.com'
354351
}
355-
}, {
356-
// setting hash to '' removes any hash
357-
url: 'http://www.google.com/#hash',
358-
set: {hash: ''},
359-
test: {
360-
hash: null,
361-
href: 'http://www.google.com/'
362-
}
363-
}, {
364-
// setting hash to '#' removes any hash
365-
url: 'http://www.google.com/#hash',
366-
set: {hash: '#'},
367-
test: {
368-
hash: null,
369-
href: 'http://www.google.com/'
370-
}
371-
}, {
372-
// setting search to '' removes any search
373-
url: 'http://www.google.com/?search',
374-
set: {search: ''},
375-
test: {
376-
search: null,
377-
href: 'http://www.google.com/'
378-
}
379-
}, {
380-
// setting search to '?' removes any search
381-
url: 'http://www.google.com/?search',
382-
set: {search: '?'},
383-
test: {
384-
search: null,
385-
href: 'http://www.google.com/'
386-
}
387352
}
388353

389354
];

0 commit comments

Comments
 (0)