feat(vercel): allow external redirects

[wotschofsky]

Based on issue #11313

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.

/cc @withastro/maintainers-docs for feedback!

[changeset-bot]

🦋 Changeset detected

Latest commit: 1463b0e

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

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

[Fryuni]

Hey @wotschofsky! Thanks for opening this PR and congrats for your fist contribution on Astro's codebase, we are glad to have you here 🎉 🎉 🎉

Sorry for the delayed review, the code overall is great and simple. I left two little notes, one for improved readability and another about a potential problem with the proposed solution along with an alternative.

[Fryuni]

This code seems correct, but it might be more readable if we didn't use mutable state. What about splitting this:

function getRedirectDestination(route: RouteData): string {
	if (route.redirectRoute) {
		return getReplacePattern(route.redirectRoute.segments);
	}
	if (typeof route.redirect === 'object') {
		return route.redirect.destination;
	}
	return route.redirect || '';
}

function getRedirectLocation(route: RouteData, config: AstroConfig): string {
	const redirectDestination = getRedirectDestination(route);
	const forceTrailingSlash = config.trailingSlash === 'always';

[wotschofsky]

Currently the trailingSlash rule is only considered in the first case. However I don't see a reason not to always apply it. If so forceTrailingSlash can be eliminated completely.

This version has significantly less mutable state. The only option I see for avoiding that completely, that doesn't introduce another level of complexity through a helper function, would be using ternary expression for conditionally appending the slash.

function getDestinationFromRoute(route: RouteData) {
	if (route.redirectRoute) {
		return getReplacePattern(route.redirectRoute.segments);
	}
	if (typeof route.redirect === 'object') {
		return route.redirect.destination;
	}
	return route.redirect || '';
}

function getRedirectLocation(route: RouteData, config: AstroConfig): string {
	let destination = getDestinationFromRoute(route);

	// Is a full URL - do not transform
	if (URL.canParse(destination)) {
		return destination;
	}

	if (config.trailingSlash === 'always') {
		destination = appendForwardSlash(destination);
	}

	return pathJoin(config.base, destination);
}

[ematipico]

I don't think this is the correct way to implement the external redirects. The test (and later the docs) shows that users can have external URLs in the astro.config.mjs, which is confusing because by design Astro doesn't support external redirects.

These directs should be passed to the adapter instead.

[Fryuni]

I don't think this is the correct way to implement the external redirects. The test (and later the docs) shows that users can have external URLs in the astro.config.mjs, which is confusing because by design Astro doesn't support external redirects.

This doesn't sound right. Astro itself, without any adapters, works with external redirects. The following configuration works exactly as the user declares when building in SSG without any adapter:

export default defineConfig({
  redirects: {
    '/foo': 'https://example.com'
  }
});

Using another adapter like Node also works and redirects to the external URL with the following config:

export default defineConfig({
  redirects: {
    '/foo': 'https://example.com'
  },
  output: "server",
  adapter: node({
    mode: "standalone"
  })
});

So it does look, from a user's perspective, like a bug on the Vercel adapter that currently would redirect to <your site>/https:/example.com. Which is fixed by this PR.

I know that this is not "officially"/"intentionally" supported (it warned as such when used), but the diverging behavior is weird.

[alexanderniebuhr]

We have moved the adapter to https://github.com/withastro/adapters, so this PR can't be merged here anymore.
I would offer to port over the changes to the new repo for you, just let me know if you want me to do that.

[Princesseuh]

Closing due to inactivity, please refer to Alex's message for what to do next.

[wotschofsky]

I've reopend the PR on the new repo: withastro/adapters#404