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

feat(url): 6 new URL helpers in new section #328

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

baxyz
Copy link

@baxyz baxyz commented Dec 20, 2024

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,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed
  • Release notes in next-minor.md or next-major.md have been added, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1
A src/url/cleanPath.ts 86
A src/url/onlyPath.ts 89
A src/url/withLeadingSlash.ts 80
A src/url/withTrailingSlash.ts 90
A src/url/withoutLeadingSlash.ts 89
A src/url/withoutTrailingSlash.ts 102

Footnotes

  1. Function size includes the import dependencies of the function.

@baxyz baxyz requested a review from aleclarson as a code owner December 20, 2024 22:37
@baxyz baxyz changed the title URL new section with 6 helpers 6 new URL helpers in new section Dec 20, 2024
@baxyz baxyz changed the title 6 new URL helpers in new section feat(url): 6 new URL helpers in new section Dec 20, 2024
@aleclarson
Copy link
Member

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.

  1. What makes your functions essential when that API already exists? Are any of them already achievable using just the URL web API?
  2. In what contexts are these functions indispensable? How do you envision them being used?

@MarlonPassos-git MarlonPassos-git added new feature This PR adds a new function or extends an existing one awaiting more feedback Wait on this until more people comment. labels Dec 27, 2024
@radashi-bot
Copy link

radashi-bot commented Jan 4, 2025

Benchmark Results

Name Current
cleanPath: with no input 4,890,426.27 ops/sec ±0.09%
cleanPath: with empty string 2,391,539.37 ops/sec ±0.09%
cleanPath: with correct path 2,327,203.91 ops/sec ±0.09%
cleanPath: with multiple slashes in path 1,713,339.25 ops/sec ±0.84%
cleanPath: with protocol, path, query, and fragment 1,572,555.76 ops/sec ±0.2%
onlyPath: with no input 4,792,134.97 ops/sec ±0.16%
onlyPath: with empty string 4,145,612.41 ops/sec ±0.09%
onlyPath: with path only 3,389,365.11 ops/sec ±0.98%
onlyPath: with path and query 2,923,477.04 ops/sec ±0.25%
onlyPath: with path and fragment 2,930,367.62 ops/sec ±0.17%
onlyPath: with path, query, and fragment 2,594,427.67 ops/sec ±0.62%
withLeadingSlash: with no input 4,819,625.55 ops/sec ±0.1%
withLeadingSlash: with empty string 4,843,431.66 ops/sec ±0.1%
withLeadingSlash: with missing leading slash 4,811,217.62 ops/sec ±0.1%
withLeadingSlash: with leading slash 4,776,107.6 ops/sec ±0.19%
withTrailingSlash: with no input 4,788,417.73 ops/sec ±0.09%
withTrailingSlash: with empty string 3,632,096.65 ops/sec ±0.19%
withTrailingSlash: with missing trailing slash 4,739,054.3 ops/sec ±0.09%
withTrailingSlash: with trailing slash 4,668,297.29 ops/sec ±0.09%
withoutLeadingSlash: with no input 4,753,713.25 ops/sec ±0.1%
withoutLeadingSlash: with empty string 4,662,391.71 ops/sec ±0.1%
withoutLeadingSlash: with leading slash 4,666,499.08 ops/sec ±0.22%
withoutLeadingSlash: without leading slash 4,649,770.31 ops/sec ±0.1%
withoutTrailingSlash: with no input 4,950,665.53 ops/sec ±0.1%
withoutTrailingSlash: with empty string 3,664,201.59 ops/sec ±0.09%
withoutTrailingSlash: with trailing slash 4,729,229.54 ops/sec ±0.1%
withoutTrailingSlash: without trailing slash 4,609,182.69 ops/sec ±0.17%

Performance 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.

@baxyz
Copy link
Author

baxyz commented Jan 4, 2025

@aleclarson Oh, great questions!

I see two main use cases:

  • Legacy: The current URL's details (host, path, query, hash, etc.) are provided in string.
  • Routing in SPA: Handling paths textually is faster in single-page applications.

Some additional reasons:

  • Simpler and more straightforward to use compared to the URL API, which can be overkill for straightforward tasks.
  • Compatibility
  • Performance
  • Handling incomplete URLs / Flexibility

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
Note: not easy to use the URL API only to clean paths in templates.

<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.
new URL(x).toString() is less readable than clearPath(x), and more obscure.

Does it make any sense? What are your thoughts?

@aleclarson
Copy link
Member

Hey @baxyz, thank you for the detailed explanation and examples!

After reviewing your use cases, I have a couple of thoughts to share:

  • These utilities seem primarily focused on frontend/DOM scenarios, whereas Radashi core aims to be runtime-agnostic. For frontend-specific path handling, we're actually exploring a dedicated DOM/Web package (see discussion #197) which might be a better home for these utilities.
  • While I understand the appeal of simpler path manipulation functions, I have some concerns about encouraging "path cleaning" as a pattern. In my experience, when encountering malformed paths (e.g., duplicate slashes), it's often more maintainable to fix the issue at its source rather than normalizing downstream. These utilities, while functional, might mask underlying issues that should be addressed directly.

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.

@baxyz
Copy link
Author

baxyz commented Jan 5, 2025

@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:

  1. There's a bit of confusion between my examples and your scope (runtime-agnostic): these utilities are not related to the DOM (no direct or indirect link), and they're not restricted to the frontend. For instance, one could have an ExpressJS setup using a helper (hence, on the backend in Node).

    const app = require('express')();
    app.on(withLeadingSlash(keyword), (req, res) => {
        res.send('Hello World');
    });
  2. These utilities are essentially string manipulations, similar to functions like string.capitalize() or object.invert(), especially the withXxxSlash and withoutXxxSlash.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting more feedback Wait on this until more people comment. new feature This PR adds a new function or extends an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants