-
Notifications
You must be signed in to change notification settings - Fork 0
fix: remove doubled slash when using URL instance #35
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,7 +96,30 @@ function url(strings, ...args) { | |
|
|
||
| return encodeComponent(raw) | ||
| }) | ||
| const res = [strings[0], ...escaped.flatMap((arg, i) => [arg, strings[i + 1]])].join('') | ||
|
|
||
| // Handle the case where a URL object is used as base and creates double slashes | ||
| let res | ||
|
|
||
| if (base && escaped.length > 0) { | ||
| const parts = [strings[0]] | ||
|
|
||
| for (const [i, escapedArg] of escaped.entries()) { | ||
| parts.push(escapedArg) | ||
| let nextString = strings[i + 1] | ||
|
|
||
| // If the previous part ends with '/' and next string starts with '/', remove the duplicate | ||
| if (parts[parts.length - 1].endsWith('/') && nextString.startsWith('/')) { | ||
|
garana-exodus marked this conversation as resolved.
|
||
| nextString = nextString.slice(1) | ||
| } | ||
|
|
||
| parts.push(nextString) | ||
| } | ||
|
|
||
| res = parts.join('') | ||
| } else { | ||
| res = [strings[0], ...escaped.flatMap((arg, i) => [arg, strings[i + 1]])].join('') | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the impl looks too complex
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I echo's Gonzalo's question above: why do we need to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can use URL. Just don't do something like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll update the clients to change it. Although, we should still land this fix or remove |
||
| if (base) validateBase(base, res) | ||
| const url = new URL(res) | ||
| if (String(url) !== res) throw new Error('Unexpected URL produced!') // e.g. .. which get resolved | ||
|
|
||
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.
Perhaps this has already been discussed: why not use the native
URLobject? It should account for//in the path.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 can't remember why we added
url, maybe @sparten11740 or @ChALkeR knows better