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

Astro.rewrite encodes path into url causes dynamic params mismatch #13098

Closed
1 task done
unprintable123 opened this issue Jan 30, 2025 · 3 comments · Fixed by #13113
Closed
1 task done

Astro.rewrite encodes path into url causes dynamic params mismatch #13098

unprintable123 opened this issue Jan 30, 2025 · 3 comments · Fixed by #13113
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: routing Related to Astro routing (scope)

Comments

@unprintable123
Copy link
Contributor

Astro Info

Astro                    v5.2.0
Node                     v20.15.0
System                   Windows (x64)
Package Manager          yarn
Output                   static
Adapter                  none
Integrations             @astrojs/vue
                         @astrojs/mdx

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

[ERROR] [NoMatchingStaticPathFound] A `getStaticPaths()` route pattern was matched, but no matching static path was found for requested path `/ABC%20abc%20123`.

Astro.rewrite will encode the path into url but the dynamic params won't decode it.

The patch works for me but i don't know the intended usage of pathname and whether it will crash somewhere else.

--- astro/src/core/routing/rewrite.ts
+++ astro/src/core/routing/rewrite.ts
@@ -36,7 +36,7 @@
     return {
       routeData: foundRoute,
       newUrl,
-      pathname
+      pathname: decodeURI(pathname)
     };
   } else {
     const custom404 = routes.find((route) => route.route === "/404");

What's the expected result?

rewrite should use decoded path to match the dynamic params.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-dh2hv9pz?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 30, 2025
@unprintable123 unprintable123 changed the title Astro.rewrite encodes path into url causes params mismatch Astro.rewrite encodes path into url causes dynamic params mismatch Jan 30, 2025
@ematipico
Copy link
Member

ematipico commented Jan 30, 2025

URLs are decoded at the source now, not inside the rendering phase (rewrite is a rendering phase).

This means that users are responsible for encoding/decoding.

The fix won't work for some cases, I need to understand the use case and what we can do to fix it.

@unprintable123
Copy link
Contributor Author

I want to rewrite /tag/[tag] to /tag/[tag]/page/1. The tag may contain non-ascii letters and during the rewrite it will be encoded. So in dynamic params it uses encoded tag to lookup the raw tag and causes mismatch.

The problem is that I can't rewrite to any dynamic routing path with non-ascii letters.

The code at L58 uses decoded pathname to match the routing, but in getProps L54 it is not decoded.
So I think there should be somewhere to decode the path.

@ematipico ematipico added - P4: important Violate documented behavior or significantly impacts performance (priority) feat: routing Related to Astro routing (scope) labels Jan 31, 2025
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Jan 31, 2025
@ematipico
Copy link
Member

@unprintable123 I think your suggested fix makes sense, would you send a PR with a test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: routing Related to Astro routing (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants