-
Notifications
You must be signed in to change notification settings - Fork 27
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
feat(url): 6 new URL helpers in new section #328
base: main
Are you sure you want to change the base?
Conversation
Thanks for the PR! 🎉 I assume you're aware of the URL standard Web API, which backend runtimes (e.g. Node.js, Bun, Deno) also support. With that in mind, I'd like to better understand your thinking here.
|
Benchmark ResultsPerformance regressions of 30% or more should be investigated, unless they were anticipated. Smaller regressions may be due to normal variability, as we don't use dedicated CI infrastructure. |
@aleclarson Oh, great questions! I see two main use cases:
Some additional reasons:
Here are some example uses: OAuth2: Calculating the "issuer" with the host const issuer = cleanPath(`${window.location.protocol}//auth.${window.location.host}/realm/${realm}`); SPA: Quick paths for routing <a [href]="withLeadingSlash(path0)">root page</a> <!-- need "/path", not "path" -->
<a [href]="withoutLeadingSlash(path1)">sibling page</a> <!-- need "path", not "/path" -->
<a [href]="withoutTrailingSlash(path2)">page, not a resource</a> <!-- need "path", not "path/" --> Clear path: const apiEndpoint = cleanPath(`https://api.example.com/${version}/user/${userId}`);
const normalizedPath = cleanPath('/user//files///docs/file.txt'); versus const url = new URL(`https://api.example.com/${version}/user/${userId}`);
const apiEndpoint = url.toString();
const basePath = new URL('file://');
basePath.pathname = '/user//files///docs/file.txt';
const normalizedPath = basePath.pathname; IMHO, that's clearer to use string helpers rather than relying on the URL API that, according to the documentation, should perform the task but doesn’t explicitly do it. Does it make any sense? What are your thoughts? |
Hey @baxyz, thank you for the detailed explanation and examples! After reviewing your use cases, I have a couple of thoughts to share:
That said, I'm open to community input on this. If other maintainers and users see value in having these utilities in core, I'm willing to reconsider. I really appreciate the thought and effort you've put into this PR, especially in documenting the use cases so thoroughly. Please don't let these concerns discourage you from future contributions! And for future PRs, feel free to wait for initial review before investing time in detailed docs and benchmarks. |
@aleclarson No worries if this PR doesn't make it through; I had that possibility in mind before starting. 😄 However, I'd like to clarify two things:
I've been using these types of methods for several years, but I understand that they might not interest the community. Maybe this last point can be discussed further in the continuation of discussion #279 Does it make any sense? What are your thoughts? |
Summary
This pull request introduces a new category "URL" and adds six new utility functions for URL manipulation:
cleanPath
: Removes duplicate slashes from a URL.onlyPath
: Extracts only the path from a URL, removing any query or fragment.withLeadingSlash
: Adds a leading slash to a URL if it is not already present.withoutLeadingSlash
: Removes the leading slash from a URL if it is present.withTrailingSlash
: Adds a trailing slash to a URL if it is not already present.withoutTrailingSlash
: Removes the trailing slash from a URL if it is present.Related issue, if any:
There is no related issue.
This is a merge of the Helpers4 library and new helpers.
Cf. discussion #279
For any code change,
Does this PR introduce a breaking change?
No
Bundle impact
src/url/cleanPath.ts
src/url/onlyPath.ts
src/url/withLeadingSlash.ts
src/url/withTrailingSlash.ts
src/url/withoutLeadingSlash.ts
src/url/withoutTrailingSlash.ts
Footnotes
Function size includes the
import
dependencies of the function. ↩