From 06ec9e79168b3e6bc1c1ad5db416a6abf899f740 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Wed, 28 Jan 2015 10:50:14 -0800 Subject: [PATCH] Revert "url: support `path` for url.format" This reverts commit d312b6d15c69cf4c438ed7d884e6396c481a57f6. d312b6d15c69cf4c438ed7d884e6396c481a57f6 introduced some confusion in the existing API of url.format and url.parse. The way the 'path' property overrides other properties in url.format's input is too confusing for existing users compared to the issues it fixes. Fixes such as https://github.com/joyent/node/pull/9081 have been proposed, but they do not make the API less confusing. Instead, this change just reverts the original breaking change so that it gives us more time after v0.12.0 is released to come up with a better API for url.format, url.parse and other related APIs in the v0.13 development branch. Fixes #9070. Conflicts: doc/api/url.markdown PR: #9109 PR-URL: https://github.com/joyent/node/pull/9109 Reviewed-By: Timothy J Fontaine --- doc/api/url.markdown | 1 - lib/url.js | 31 +++-------------- test/simple/test-url.js | 77 +++-------------------------------------- 3 files changed, 9 insertions(+), 100 deletions(-) diff --git a/doc/api/url.markdown b/doc/api/url.markdown index 39953fba591c..280f44ae6815 100644 --- a/doc/api/url.markdown +++ b/doc/api/url.markdown @@ -98,7 +98,6 @@ Here's how the formatting process works: * `port` will only be used if `host` is absent. * `host` will be used in place of `hostname` and `port` * `pathname` is treated the same with or without the leading `/` (slash). -* `path` is treated the same with `pathname` but able to contain `query` as well. * `search` will be used in place of `query`. * It is treated the same with or without the leading `?` (question mark) * `query` (object; see `querystring`) will only be used if `search` is absent. diff --git a/lib/url.js b/lib/url.js index 6c9aba3f3789..ac712318bcde 100644 --- a/lib/url.js +++ b/lib/url.js @@ -362,7 +362,7 @@ Url.prototype.parse = function(url, parseQueryString, slashesDenoteHost) { } // finally, reconstruct the href based on what has been validated. - this.href = this.format(parseQueryString); + this.href = this.format(); return this; }; @@ -377,7 +377,7 @@ function urlFormat(obj) { return obj.format(); } -Url.prototype.format = function(parseQueryString) { +Url.prototype.format = function() { var auth = this.auth || ''; if (auth) { auth = encodeURIComponent(auth); @@ -389,26 +389,7 @@ Url.prototype.format = function(parseQueryString) { pathname = this.pathname || '', hash = this.hash || '', host = false, - query = '', - search = ''; - - if (this.path) { - var qm = this.path.indexOf('?'); - if (qm !== -1) { - query = this.path.slice(qm + 1); - search = '?' + query; - pathname = this.path.slice(0, qm); - } else { - if (parseQueryString) { - this.query = {}; - this.search = ''; - } else { - this.query = null; - this.search = null; - } - pathname = this.path; - } - } + query = ''; if (this.host) { host = auth + this.host; @@ -421,15 +402,13 @@ Url.prototype.format = function(parseQueryString) { } } - if (!query && - this.query && + if (this.query && util.isObject(this.query) && Object.keys(this.query).length) { query = querystring.stringify(this.query); } - if (!search) - search = this.search || (query && ('?' + query)) || ''; + var search = this.search || (query && ('?' + query)) || ''; if (protocol && protocol.substr(-1) !== ':') protocol += ':'; diff --git a/test/simple/test-url.js b/test/simple/test-url.js index 9eaaf0353ff8..e81908a883fa 100644 --- a/test/simple/test-url.js +++ b/test/simple/test-url.js @@ -1113,7 +1113,7 @@ var formatTests = { // `#`,`?` in path '/path/to/%%23%3F+=&.txt?foo=theA1#bar' : { - href: '/path/to/%%23%3F+=&.txt?foo=theA1#bar', + href : '/path/to/%%23%3F+=&.txt?foo=theA1#bar', pathname: '/path/to/%#?+=&.txt', query: { foo: 'theA1' @@ -1123,7 +1123,7 @@ var formatTests = { // `#`,`?` in path + `#` in query '/path/to/%%23%3F+=&.txt?foo=the%231#bar' : { - href: '/path/to/%%23%3F+=&.txt?foo=the%231#bar', + href : '/path/to/%%23%3F+=&.txt?foo=the%231#bar', pathname: '/path/to/%#?+=&.txt', query: { foo: 'the#1' @@ -1138,7 +1138,7 @@ var formatTests = { hostname: 'ex.com', hash: '#frag', search: '?abc=the#1?&foo=bar', - pathname: '/foo?100%m#r' + pathname: '/foo?100%m#r', }, // `?` and `#` in search only @@ -1148,77 +1148,8 @@ var formatTests = { hostname: 'ex.com', hash: '#frag', search: '?abc=the#1?&foo=bar', - pathname: '/fooA100%mBr' - }, - - // path - 'http://github.com/joyent/node#js1': { - href: 'http://github.com/joyent/node#js1', - protocol: 'http:', - hostname: 'github.com', - hash: '#js1', - path: '/joyent/node' - }, - - // pathname vs. path, path wins - 'http://github.com/joyent/node2#js1': { - href: 'http://github.com/joyent/node2#js1', - protocol: 'http:', - hostname: 'github.com', - hash: '#js1', - path: '/joyent/node2', - pathname: '/joyent/node' - }, - - // pathname with query/search - 'http://github.com/joyent/node?foo=bar#js2': { - href: 'http://github.com/joyent/node?foo=bar#js2', - protocol: 'http:', - hostname: 'github.com', - hash: '#js2', - path: '/joyent/node?foo=bar' - }, - - // path vs. query, path wins - 'http://github.com/joyent/node?foo=bar2#js3': { - href: 'http://github.com/joyent/node?foo=bar2#js3', - protocol: 'http:', - hostname: 'github.com', - hash: '#js3', - path: '/joyent/node?foo=bar2', - query: {foo: 'bar'} - }, - - // path vs. search, path wins - 'http://github.com/joyent/node?foo=bar3#js4': { - href: 'http://github.com/joyent/node?foo=bar3#js4', - protocol: 'http:', - hostname: 'github.com', - hash: '#js4', - path: '/joyent/node?foo=bar3', - search: '?foo=bar' - }, - - // path is present without ? vs. query given - 'http://github.com/joyent/node#js5': { - href: 'http://github.com/joyent/node#js5', - protocol: 'http:', - hostname: 'github.com', - hash: '#js5', - path: '/joyent/node', - query: {foo: 'bar'} - }, - - // path is present without ? vs. search given - 'http://github.com/joyent/node#js6': { - href: 'http://github.com/joyent/node#js6', - protocol: 'http:', - hostname: 'github.com', - hash: '#js6', - path: '/joyent/node', - search: '?foo=bar' + pathname: '/fooA100%mBr', } - }; for (var u in formatTests) { var expect = formatTests[u].href;