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

API Route always returns 404 on Vercel depending on trailingSlash #9206

Closed
1 task
florian-lefebvre opened this issue Nov 28, 2023 · 7 comments · Fixed by #9597
Closed
1 task

API Route always returns 404 on Vercel depending on trailingSlash #9206

florian-lefebvre opened this issue Nov 28, 2023 · 7 comments · Fixed by #9597
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: ssr Related to SSR (scope)

Comments

@florian-lefebvre
Copy link
Member

Astro Info

Astro                    v3.6.1
Node                     v18.14.1
System                   Windows (x64)
Package Manager          yarn
Output                   hybrid
Adapter                  @astrojs/vercel/serverless
Integrations             none

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

No response

Describe the Bug

When setting trailingSlash in both astro and vercel configs, I think there is some conflict that causes API routes to always be 404.

What's the expected result?

The route /api/test/ should return some json. Here is the deployment: https://astro-vercel-endpoint-issue.vercel.app/api/test/

Link to Minimal Reproducible Example

https://github.com/florian-lefebvre/astro-vercel-endpoint-issue

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 Nov 28, 2023
@lilnasy lilnasy added - P4: important Violate documented behavior or significantly impacts performance (priority) pkg: vercel Related to Vercel adapter (scope) pkg: node Related to Node adapter (scope) feat: ssr Related to SSR (scope) and removed needs triage Issue needs to be triaged pkg: vercel Related to Vercel adapter (scope) pkg: node Related to Node adapter (scope) labels Nov 30, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Nov 30, 2023

Does not just affect vercel. The combination of base and trailingSlash is sufficient to break routing.

defineConfig({
    base: "/",
    trailingSlash: "always"
})

@lilnasy
Copy link
Contributor

lilnasy commented Nov 30, 2023

i am not sure if base: "/" even makes sense
i think the fix would be to error when base ends in slash or ignore base's trailing slash 💀

@florian-lefebvre
Copy link
Member Author

I thought / would kinda mean root but yeah, I guess we should always strip base's trailing slash

@florian-lefebvre
Copy link
Member Author

I checked and removing base does not solve the issue

@matthewp matthewp self-assigned this Dec 28, 2023
@matthewp
Copy link
Contributor

Is output: 'hybrid' required to see this problem @florian-lefebvre ?

@lilnasy
Copy link
Contributor

lilnasy commented Jan 3, 2024

This issue is that astro's trailingSlash never acts on API routes. They are always served without it. When you enable vercel's trailingSlash, it redirects from /api/test (which astro could have served) to /api/test/ (which astro currently does not; bug).

We could make trailingSlash: "always" apply to API routes but that would be a breaking change because currently it only works without them. Since the option is for SEO use-cases, API routes have some wiggle room - we could ignore the trailing slash in all cases.

@MadSpindel
Copy link

I think I've a similar issue with /_image and Vercel. If I enable trailingSlash: true in vercel.json, all images stop working on SSR pages, since Vercel is changing the /_image URL to /_image/. And it looks like trailingSlash: 'always' doesn't affect the /_image endpoint?

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: ssr Related to SSR (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants