Skip to content

refactor doc route url parsing #191

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

Merged
merged 3 commits into from
Jun 23, 2025
Merged

Conversation

brookslybrand
Copy link
Contributor

After merging #190 I realized we had a bug with old tags

For example, if you add .md to the end of https://reactrouter.com/6.30.1/start/overview, the redirect will not work properly

The fix is corrected on staging, so https://reactrouterdotcomstaging.fly.dev/6.30.1/start/overview.md works

Additionally, I went ahead and added (ok ok, cursor added) some tests to ensure that we don't break the behavior in the future.

These tests made it easy to refactor the url parsing logic, which is still a bit hairy, but much more tame now.

@JuanJo4 if you have time I'd love a review since you implemented the original feature :)

@JuanJo4
Copy link
Contributor

JuanJo4 commented Jun 19, 2025

@brookslybrand ohhh that's a good catch! the tests look good and definetly helps to understand all of that url parsing logic, and that refactoring make it a lot easier to read through the code.

LGTM ✅

@brookslybrand brookslybrand merged commit 79d15e6 into main Jun 23, 2025
5 checks passed
@brookslybrand brookslybrand deleted the brooks/refactor-doc-url-parsing branch June 23, 2025 12:50
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

Successfully merging this pull request may close these issues.

2 participants