defunctzombie / node-url

node.js core url module as a module
MIT License
376 stars 96 forks source link

url.format() should normalize multiple slashes in pathname to single slashes #27

Closed prometheas closed 7 years ago

prometheas commented 7 years ago

I can't think of an instance when it's actually valuable / desired that a URL ever gets rendered with multiple consecutive slashes, like /path//to/something, which is an occurrence that might come up when building concatenating strings to build a path.

I'm prepared to submit a PR for this, but I wanted to run the idea by the maintainer to determine whether there were any objections and to confirm that a PR would be welcome.

aearly commented 7 years ago

What does node's built in url.format() do? node-url tries to match the built-in behavior as much as possible.

Download commented 7 years ago

Some apps use path parameters.

E.g:

get('/:type/:customer/:year', function handle(req, res){
  // ...
})

Changing multiple slashes to single slashes would change the meaning of the URL:

invoice//2017 => All invoices for all customers in 2017 invoice/2017 => No invoice found for customer 2017

So I don't think it's a good idea.

prometheas commented 7 years ago

Very fair point, @aearly. I can look into that.

Whoever designed that user-hostile route should really learn about RESTful design. And, you know, query strings 🤦‍♂️ that said, I'm entirely sure that you're frighteningly, depressingly correct about that, @Download. At least, I suppose, we can be thankful that this hypothetical web service doesn't simply use / and rely completely on HTTP POST data to pass :type, :customer, and :year values 😉

Download commented 7 years ago

I'm entirely sure that you're frighteningly, depressingly correct about that, @Download.

Ha ha yes it is a bit depressing isn't it? Query string for the win!

prometheas commented 7 years ago

Actually, I'm realizing that—having now gotten all projects I help maintain onto modern versions of node—I no longer have a stake in solving this 😄

I'm going to close it out (obviously feel free to reopen if my doing so was premature).