This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

Revert "url: support `path` for url.format"

This reverts commit d312b6d.

d312b6d 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 #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: #9109
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
  • Loading branch information...
Julien Gilli
Julien Gilli committed Jan 28, 2015
1 parent 491ac6a commit 3b392d33c7c2e9ec143e3e8fd0a00c242b9342aa
Showing with 9 additions and 100 deletions.
  1. +0 −1 doc/api/url.markdown
  2. +5 −26 lib/url.js
  3. +4 −73 test/simple/test-url.js
@@ -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.
@@ -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 += ':';

@@ -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;

0 comments on commit 3b392d3

Please sign in to comment.