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

Trailing slash is not preserved #10

Open
freeqaz opened this issue Feb 17, 2016 · 7 comments
Open

Trailing slash is not preserved #10

freeqaz opened this issue Feb 17, 2016 · 7 comments
Labels

Comments

@freeqaz
Copy link

freeqaz commented Feb 17, 2016

var urlJoin = require('url-join');
var joinedUrl = urlJoin('www.example.com', '/test/?hello=true');
console.log(joinedUrl);

Expected output:
'www.example.com/test/?hello=true'

Actual output:
'www.example.com/test?hello=true'

This causes issues for SEO where having 2 different URLs leads to fragmentation in search results.

@rauberdaniel
Copy link
Contributor

I think the problem with this one is that (imho) there currently is no easy way to add options to url-join and I think keepTrailingSlash should definitely be optional. @freeqaz What’s the reason you don’t just redirect everything to not using the trailing slash?

@ArmorDarks
Copy link

I think situation is easier.

'/test/?hello=true' contains / before ?, so it should be preserved as whole fragment. It's logical, expected behavior.

@ArmorDarks
Copy link

Another approach would be to disallow any queries in url at all and pass them via option, so that library would never have issues with trying to parse everything we feed to it.

I think it's quite viable approach, since it works in React Router quite well, see https://github.com/ReactTraining/react-router/blob/master/docs/API.md#link

@bdrobinson
Copy link

Any news on this? This caught us out and feels like very unexpected behaviour.

I see that adding an options param to the main urljoin function isn't really possible. So how about adding a second exported function joinWithOptions (or something along those lines) where the first arg is an array and the second arg is an options object with a preserveTrailingSlash property?

It would also be good to have something in the README explaining that the trailing slash will get removed, as this behaviour is currently undocumented? (as far as I can see)

@Arrvi
Copy link

Arrvi commented May 9, 2019

This problem still occurs and requires us to do workarounds.

@limeandcoconut
Copy link

@ArmorDarks has the right of it. Both RFC 3986 and the WhatHG URL standard allow trailing slashes here. The correct behavior is to preserve it if it exists. I'm gonna have to fork since this has gone unfixed for 6years.

@jonkoops
Copy link
Collaborator

@limeandcoconut instead of forking this repo perhaps you can submit a PR to fix this? I'd be more than willing to review it for you and get it merged.

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

No branches or pull requests

7 participants