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

is url.format broken or am I? #1176

Closed
beck opened this issue Mar 21, 2018 · 7 comments
Closed

is url.format broken or am I? #1176

beck opened this issue Mar 21, 2018 · 7 comments

Comments

@beck
Copy link

beck commented Mar 21, 2018

Using Node@9.9 on OSX.

Problem

> const url = require('url')
> url.format({hostname: 'dougbeck.me', pathname: 'womp.html'})
'dougbeck.mewomp.html'
> 😭

Am I crazy for thinking this is broken behavior?

Granted, this seems to be terrible in every other language:

Python

>>> urllib.parse.urlunparse(('', 'dougbeck.me', 'womp.html', '', '', ''))
'//dougbeck.me/womp.html'

Requiring a 6-item tuple is gross.

Ruby

> URI::HTTP.build(host: 'dougbeck.me', path: 'womp.html').to_s
ERROR expected absolute path

Java

> new URL("dougbeck.me").toString()
ERROR: no protocol

Node's behavior is worse than all of ☝️because it does not throw an error and returns a result that defies all intuition. 😖

@devsnek
Copy link
Member

devsnek commented Mar 21, 2018

url.format is legacy, you should use the whatwg URL api

@addaleax
Copy link
Member

👍 to what @devsnek said

But also, yes, this is terrible and we should probably fix it. Would you be willing to open an issue @ https://github.com/nodejs/node/, for visibility?

@beck
Copy link
Author

beck commented Mar 21, 2018

you should use the whatwg URL api

Lets give that a shot...

> const url = require('whatwg-url')
> url.serializeURL({host: 'dougbeck.me', path: 'womp.html'})
'undefined://undefined:undefined@dougbeck.me:undefined/w/o/m/p/./h/t/m/l?undefined#undefined'

¡Dios mííío!

whatwg-url is great when starting with a url string. Digging through the source, I have no clue how to create a URL object or url string from parts. For my use case, there is no starting url string since some parts are sourced from config.

I'm being a bit facetious, it is super easy to build a url with string munging or a template string but I'd very much like to use a lib and be confident I'll never see a double slash.

I'm on the hunt for a lib that accepts:

  • domain
  • path
  • query dictionary

and returns:

  • a fetchable url string
  • where query params are properly concatenated and url encoded
  • and free of //

Maybe I'm asking for too much.

@ORESoftware
Copy link

ORESoftware commented Mar 21, 2018

Yeah I guess it should be:

> const url = require('url')
> url.format({hostname: 'dougbeck.me', pathname: '/womp.html'});  // with a slash
'dougbeck.me/womp.html'

but I agree that the slash should not be necessary, that would nice. this does seem problematic... these url parsing things can be a pain in the arse in any language. I also agree that it should throw if there is an obvious formatting problem (in the next Node.js major version).

@addaleax
Copy link
Member

@beck I think the intention was not to point to that module, but rather to https://nodejs.org/api/url.html#url_the_whatwg_url_api

const url = new URL('https://./');  // Dummy HTTPS URL
url.hostname = 'foo';
url.pathname = 'bar';
url.search = new URLSearchParams({a: 'b'});
console.log(url.href);  // => 'https://foo/bar?a=b'
// console.log(url);

@jasnell
Copy link
Member

jasnell commented Mar 21, 2018

The point being, the WHATWG URL API is now implemented in core and is generally available and should be what is used.

@beck
Copy link
Author

beck commented Mar 21, 2018

to open an issue for visibility

nodejs/node#19523

@addaleax the dummy url is ugly but good. Minor edit would be to document via code:

const dummyURL = 'https://./';
const url = new URL(dummyURL);

I like this approach. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants