-
Notifications
You must be signed in to change notification settings - Fork 30k
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: improve WHATWG url constructor code example #32782
Conversation
## Why this PR ? Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. ## Suggested change Improve the code example that shows how the new URL constructor should be accessed to begin with.
If we want to promote accessing the new WHATWG URL from the global scope then maybe the code example as-is is ok, but otherwise, since this is a docs for the core URL module then it is somewhat confusing how to access since doing this won't work: const URL = require('url')
const myURL = new URL('..') |
@lirantal Yeah, since all supported Node.js versions have URL on the global object, I’d not document Maybe the |
Exactly, so it depends what we want to promote :-) |
@lirantal Since nobody else has weighed in here yet: I’d say let’s not do this, and instead mention this as an alternative possibility for access in the URL constructor’s documentation section. |
Sure. So would we replace this one https://github.com/nodejs/node/pull/32782/files#diff-dd182d238def46a3af0d112f617e8838L104 with the option I suggested or just add it as a note below it? I need an actual code example would be good. |
doc/api/url.md
Outdated
@@ -53,6 +53,7 @@ WHATWG URL's `origin` property includes `protocol` and `host`, but not | |||
Parsing the URL string using the WHATWG API: | |||
|
|||
```js | |||
const URL = require('url').URL |
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.
Now that URL
is global, this is not strictly required. This might be improved by explaining that distinction?
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.
I know it works global :)
I'm thinking out loud in this PR whether we want to promote that or not and yes to make one or the other more explicit so devs are aware.
@lirantal I think an explicit second example would be good. E.g.:
|
@addaleax added it to the constructor example instead of the generic one on the intro. |
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.
LGTM with the linter failure fixed (should be ```js
)
all fixed up 👍 |
CI fails on |
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Landed in 6b2e3af |
Thanks Anna 👍 |
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Currently, the URL docs for the WHATWG URL spec support are somewhat lacking in their code example of how to access the new URL constructor that lives inside the core url package. PR-URL: #32782 Reviewed-By: Anna Henningsen <[email protected]>
Why this PR ?
Currently, the URL docs for the WHATWG URL spec support are
somewhat lacking in their code example of how to access the
new URL constructor that lives inside the core url package.
Suggested change
Improve the code example that shows how the new URL
constructor should be accessed to begin with.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes