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(vercel): allow external redirects #404

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wotschofsky
Copy link

This PR was originally opened on the main Astro repo: withastro/astro#11422

Changes

Before:

Redirects with schema defined in astro.config.mjs were converted into paths on the same site: https://google.com -> /https://google.com

After:

External redirects are detected and don't have the project base path prepended.

Testing

The output (esp. .vercel/output/config.json) of building a project with the updated integration was manually inspected.

A test was added

Docs

The Astro docs generally state that supporting external links configured through astro.config.mjs isn't a goal. While the Cloudflare adapter already supports external redirects, the Vercel adapter handled external redirects differently so far. Therefore this could potentially be considered a breaking change.

Copy link

changeset-bot bot commented Sep 24, 2024

🦋 Changeset detected

Latest commit: 6c06aba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@astrojs/vercel Minor
@test/astro-vercel-basic Patch
@test/astro-vercel-function-per-route Patch
@test/astro-vercel-image Patch
@test/vercel-isr Patch
@test/vercel-max-duration Patch
@test/vercel-edge-middleware-with-edge-file Patch
@test/vercel-edge-middleware-without-edge-file Patch
@test/astro-vercel-no-output Patch
@test/astro-vercel-prerendered-error-pages Patch
@test/astro-vercel-redirects-serverless Patch
@test/astro-vercel-redirects Patch
@test/vercel-server-islands Patch
@test/astro-vercel-serverless-prerender Patch
@test/astro-vercel-serverless-with-dynamic-routes Patch
@test/astro-vercel-static-assets Patch
@test/astro-vercel-static Patch
@test/vercel-streaming Patch
@test/astro-vercel-with-speed-insights-enabled-output-as-server Patch
@test/astro-vercel-with-speed-insights-enabled-output-as-static Patch
@test/astro-vercel-with-web-analytics-enabled-output-as-static Patch
vercel-hosted-astro-project Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: vercel Related to Vercel adapter (scope) label Sep 24, 2024
@@ -18,6 +18,7 @@ describe('Redirects', () => {
},
'/blog/[...slug]': '/team/articles/[...slug]',
'/Basic/http-2-0.html': '/posts/http2',
'/google': 'https://google.com',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astro doesn't support external redirects, so adding on here is misleading because it won't work with other adapters.

Since you're adding the feature for the Vercel adapter, what about adding a new option to the adapter, maybe called externalRedirects?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not officially, but I ran into this when switching out the Cloudflare adapter, where a redirect like the one in the test works. That is what prompted me to create this PR.
Otherwise, I would strongly welcome external redirects becoming officially supported, as this should only require small adjustments, and it feels like the most intuitive approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: vercel Related to Vercel adapter (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants