-
-
Notifications
You must be signed in to change notification settings - Fork 453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add .stringifyUrl()
method
#217
Changes from 11 commits
8939d39
21a8477
b96de12
e287069
c217c58
3ebf1e6
54e4780
e76bd7b
ad0f653
a0de598
b7907b0
e661440
1a187e1
c424ee5
83449af
0a93fdb
00f4233
564d97c
75b42ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -206,3 +206,17 @@ export function stringify( | |||||
Extract a query string from a URL that can be passed into `.parse()`. | ||||||
*/ | ||||||
export function extract(url: string): string; | ||||||
|
||||||
/** | ||||||
Stringify an object into a URL with a query string and sorting the keys. | ||||||
|
||||||
@example | ||||||
``` | ||||||
queryString.stringifyUrl({url: 'https://foo.bar', query: {foo: 'bar'}}); | ||||||
// => 'https://foo.bar?foo=bar' | ||||||
``` | ||||||
*/ | ||||||
export function stringifyUrl( | ||||||
input: ParsedUrl, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
options?: StringifyOptions | ||||||
): string; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import test from 'ava'; | ||
import queryString from '..'; | ||
|
||
test('stringify URL without a query string', t => { | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar/'}), 'https://foo.bar/'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar/', query: {}}), 'https://foo.bar/'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar/#top', query: {}}), 'https://foo.bar/#top'); | ||
t.deepEqual(queryString.stringifyUrl({url: '', query: {}}), ''); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar?', query: {}}), 'https://foo.bar'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar?foo=bar', query: {}}), 'https://foo.bar?foo=bar'); | ||
}); | ||
|
||
test('stringify URL with a query string', t => { | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar', query: {foo: 'bar'}}), 'https://foo.bar?foo=bar'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar?', query: {foo: 'bar'}}), 'https://foo.bar?foo=bar'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar/#top', query: {foo: 'bar'}}), 'https://foo.bar/?foo=bar#top'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar', query: {foo: 'bar', a: 'b'}}), 'https://foo.bar?a=b&foo=bar'); | ||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar?', query: {foo: ['bar', 'baz']}}), 'https://foo.bar?foo=bar&foo=baz'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs a test to show what happens if the URL already has a query string and a query string is specified. Also add some tests that does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // @komcal |
||
t.deepEqual(queryString.stringifyUrl({url: 'https://foo.bar?foo=baz', query: {foo: 'bar'}}), 'https://foo.bar?foo=baz&foo=bar'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be surprising behavior for me. Adding another query item here with the same name actually changes the semantics as now the result would be an array instead of a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sindresorhus you think the result would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that would be completely wrong as it's a different I'm saying that it should replace instead: |
||
}); | ||
|
||
test('stringify URL from the result of `parseUrl` without query string', t => { | ||
const parsedUrl = queryString.parseUrl('https://foo.bar#top'); | ||
t.deepEqual(queryString.stringifyUrl(parsedUrl), 'https://foo.bar'); | ||
}); | ||
|
||
test('stringify URL from the result of `parseUrl` with query string', t => { | ||
const parsedUrl = queryString.parseUrl('https://foo.bar?foo=bar&foo=baz#top'); | ||
t.deepEqual(queryString.stringifyUrl(parsedUrl), 'https://foo.bar?foo=bar&foo=baz'); | ||
}); | ||
|
||
test('stringify URL from the result of `parseUrl` with query string that contains `=`', t => { | ||
const parsedUrl = queryString.parseUrl('https://foo.bar?foo=bar=&foo=baz='); | ||
t.deepEqual(queryString.stringifyUrl(parsedUrl, {encode: false}), 'https://foo.bar?foo=bar=&foo=baz='); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to document the merging behavior here.