Skip to content
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

Merged
merged 19 commits into from
Jan 16, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,13 @@ export function stringify(
Extract a query string from a URL that can be passed into `.parse()`.
*/
export function extract(url: string): string;


Copy link
Owner

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.


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No many empty lines

/**
Stringify an object of URL and the query into a URL.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be useful with an example here.

*/
export function stringifyUrl(
input: ParsedUrl,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
input: ParsedUrl,
object: ParsedUrl,

options?: StringifyOptions
): string;
23 changes: 23 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,16 @@ function removeHash(input) {
return input;
}

function getHash(url) {
let hash = '';
const hashStart = url.indexOf('#');
if (hashStart !== -1) {
hash = url.slice(hashStart);
}

return hash;
}

function extract(input) {
input = removeHash(input);
const queryStart = input.indexOf('?');
Expand Down Expand Up @@ -290,3 +300,16 @@ exports.parseUrl = (input, options) => {
query: parse(extract(input), options)
};
};

exports.stringifyUrl = (input, options) => {
const url = removeHash(input.url).split('?')[0] || '';
const queryFromUrl = this.extract(input.url);
const hash = getHash(input.url);
const stringifyQuery = this.stringify(input.query, options);
let queryString = [queryFromUrl, stringifyQuery].filter(str => str).join('&');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use acronyms. str => string

if (queryString) {
queryString = `?${queryString}`;
}

return `${url}${queryString}${hash}`;
};
15 changes: 15 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,21 @@ queryString.parseUrl('https://foo.bar?foo=bar');
//=> {url: 'https://foo.bar', query: {foo: 'bar'}}
```

### .stringifyUrl(object, options?)

The inverse of [`.parseUrl()`](https://github.com/sindresorhus/query-string#parseurlstring-options)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this, it should describe what it does.


The `object` are the same as result of `.parseUrl`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easier to just describe what it accepts specifically here.


The `options` are the same as for `.stringify()`.

Returns a URL with query string.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Returns a URL with query string.
Returns a URL with a query string.


```js
queryString.stringifyUrl({url: 'https://foo.bar', query: {foo: 'bar'}});
// => 'https://foo.bar?foo=bar'
```
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved


## Nesting

Expand Down
19 changes: 19 additions & 0 deletions test/stringify-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import test from 'ava';
import queryString from '..';

test('stringify url not containing query string', t => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('stringify url not containing query string', t => {
test('stringify URL without 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 query string', t => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('stringify url with query string', t => {
test('stringify URL with 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');
Copy link
Owner

Choose a reason for hiding this comment

The 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 parseUrl and then pass the result to stringifyUrl to make sure they are compatible.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// @komcal

});