( WIP ) fix(urls): preserve trailing slashes and normalize URL joining#18
( WIP ) fix(urls): preserve trailing slashes and normalize URL joining#18rjvkn wants to merge 5 commits intocestef:devfrom rjvkn:dev
WIP ) fix(urls): preserve trailing slashes and normalize URL joining#18Conversation
…ries before insert
…slashes when printing
WIP ) fix(urls): preserve trailing slashes and normalize URL joining
|
The approach we were taking up until now indeed seemed a bit unstable, hmu whenever this PR is ready ! |
|
You're absolutely right — I’ve paused the PR and marked it as WIP to rethink the approach. This PR tackles three different issues, but I assume the one that felt unstable to you is the "Preserve trailing slash" part. I also don’t like how manual and brittle the slash handling logic has become — it’s messy and easy to get wrong. The underlying motivation is that:
…are not the same, yet the previous version would normalize both to just I’ll take another pass to find a more robust, less fragile way to handle this without all the slash juggling. Let me know if you were referring to something else as unstable — really appreciate the review and feedback! |
|
Yep we're clearly on the same page, this is pretty much what I meant! lmk if you need any help with this 😄 |
|
Hey! Yes, I do have something to discuss, but I’m a bit tied up at the moment. I’ll get back to you at the first chance I get. Thanks for the offer! 😊 |
|
Hi @cestef, it's been a while since I last had a chance to look at this. To keep things fresh and clear, I’ll close this PR for now and revisit the approach with a clean slate.
So yeah, in the end, messing with slashes just isn’t a good idea. :) |
Description
This PR resolves three issues related to URL handling:
🛠 1. Preserve trailing slash in path components
When splitting URL paths, trailing slashes were previously lost — leading to loss of semantic distinction between endpoints like
/adminand/admin/. These can represent different resources (e.g. route vs. directory), so we now preserve the trailing slash on the last component if it existed in the original path.Commit:
🔧 2. Normalize base URL and word joining
Incorrect handling of slashes between a base URL and a word (e.g.
/admin+login) could result in malformed URLs like/adminloginor/admin//login. This change trims the trailing slash from the base and the leading slash from the word before joining them with a single slash.Commit:
🚫 3. Avoid duplicate handling of the same URL in recursive mode
In recursive mode, the same URL could be inserted and handled multiple times. This introduces a check to avoid redundant processing and handler calls.
Commit:
✅ Result
/admin+login→/admin/login/admin/+login→/admin/login/admin+/login→/admin/login/admin/+/login→/admin/login/admin/path will now be stored as["admin/"]/adminpath will now be stored as["admin"]This preserves semantic meaning, improves correctness when mapping URL paths and constructing tasks, and prevents unnecessary re-processing.
🧪 Test Coverage
Tested manually with paths:
/install//install/api//api/v1//api/v1Confirmed correct insertion, dispatch, and uniqueness under recursion.