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

doc: add URL.format() example #18888

Closed
wants to merge 4 commits into from
Closed
Changes from 1 commit
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
18 changes: 18 additions & 0 deletions doc/api/url.md
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,24 @@ changes:
The `url.format()` method returns a formatted URL string derived from
`urlObject`.

Example:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You can remove Example: too. It's obvious that it is an example.


```js
const URL = require('url');
Copy link
Member

Choose a reason for hiding this comment

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

Although it appears in other examples in this doc, I think in general we tend to leave out the require() for the module. I'm fine either way.

Copy link
Member

Choose a reason for hiding this comment

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

Oh! But we do use url and reserve URL for the newer url.URL. So maybe remove it and use url rather than URL below.


URL.format({
protocol: 'https',
hostname: `example.com`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the backticks intended? If so, are not they a bit confusing here? With them, a template literal/a tag function/other quotes inside are usually expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, @vsemozhetbyt. Backticks were not intentional there. Fixed.

pathname: '/some/path',
query: {
page: 1,
format: 'json'
}
});

// => 'https://example.com/some/path?page=1&format=json'
Copy link
Member

Choose a reason for hiding this comment

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

Other places in the file use Prints or just show the output below. I personally think a "Prints" would be better than =>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a search in the docs and found several instances of // => symbolizing a returned value. Also, this method doesn't actually print a value, it just returns it.

Copy link
Contributor Author

@zeke zeke Feb 21, 2018

Choose a reason for hiding this comment

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

I would be open to just doing // https://.... That was another "convention" I found when looking at existing code examples in Node's docs.

Copy link
Member

Choose a reason for hiding this comment

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

In this case I only spoke about this specific document. And yes, // => is still used some times but it is not really common as far as I know.

So I personally would say using // 'https:... would be good.

```

If `urlObject` is not an object or a string, `url.format()` will throw a
[`TypeError`][].

Expand Down