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

Different results between 1.19.1 and 1.19.2 when using uri.scheme #391

Open
neil-morrison44 opened this issue Nov 1, 2019 · 4 comments
Open

Comments

@neil-morrison44
Copy link

When running

  const uri = urijs(href)
  console.log(href, defaultScheme)
  uri.scheme(defaultScheme)
  console.log(uri.toString())

in version 1.19.1 the console reads:

example.com https
https://example.com

Which is what's intended, however, in version 1.19.2 it reads:

example.com https
https:///example.com

looks like there's an extra / getting in there somehow?

@rodneyrehm
Copy link
Member

duplicate of #390

@neil-morrison44
Copy link
Author

I've now read through #390

If this came as part of a major version release where it was documented that protocol-less URLs would be interpreted as relative paths that'd be fine - but as a patch it seems like a bug that's affecting how people are using the library (rightly or wrongly)

@dcsaszar
Copy link

dcsaszar commented Nov 14, 2019

Unfortunately this change also breaks our lib (https://www.npmjs.com/package/scrivito). We use the following dependency: "urijs": "^1.19.1".

With that said, from my potentially biased point of view, I agree with @neil-morrison44:
This change would better be released in a major version update (2.x).
I'd also like to see a 1.19.3 with this change reverted.

Some more detail:

This issue has a very subtle consequence in our case: While some IE11, and all other browsers treat the triple slash like a double slash, some IE11 fail with Unable to connect to the target server (see http://szborows.blogspot.com/2014/12/angularjs-cors-ie11-invalid-url.html)

@simskij
Copy link

simskij commented Feb 3, 2020

Still no rollback of this change? This is in no way a patch but a breaking change.

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

4 participants