Skip to content

Commit

Permalink
fix(redirects): attempt to get only params in dev mode (#8647)
Browse files Browse the repository at this point in the history
* fix(redirects): attempt to get only params in dev mode

* fixtures/ssr-redirect => fixtures/redirects

* add tests

* Update pnpm-lock.yaml
  • Loading branch information
lilnasy authored Sep 27, 2023
1 parent e797b68 commit 408b50c
Show file tree
Hide file tree
Showing 15 changed files with 73 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-numbers-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixed an issue where configured redirects with dynamic routes did not work in dev mode.
5 changes: 5 additions & 0 deletions packages/astro/src/core/render/params-and-props.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { ComponentInstance, Params, Props, RouteData } from '../../@types/astro.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import type { Logger } from '../logger/core.js';
import { routeIsRedirect } from '../redirects/index.js';
import { getParams } from '../routing/params.js';
import { RouteCache, callGetStaticPaths, findPathItemByKey } from './route-cache.js';

Expand All @@ -25,6 +26,10 @@ export async function getParamsAndProps(opts: GetParamsAndPropsOptions): Promise
// This is a dynamic route, start getting the params
const params = getRouteParams(route, pathname) ?? {};

if (routeIsRedirect(route)) {
return [params, {}]
}

validatePrerenderEndpointCollision(route, mod, params);

// During build, the route cache should already be populated.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
export function getStaticPaths() {
return [{ params: { dynamic:'hello' } }]
}
---
{JSON.stringify(Astro.params)}

<a href="/">home</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
export function getStaticPaths() {
return [{ params: { dynamic:'hello', route:'world' } }]
}
---
{JSON.stringify(Astro.params)}

<a href="/">home</a>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
export function getStaticPaths() {
return [{ params: {spread:'welcome/world'} }]
}
---
{JSON.stringify(Astro.params)}

<a href="/">home</a>
38 changes: 33 additions & 5 deletions packages/astro/test/redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('Astro.redirect', () => {
describe('output: "server"', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-redirect/',
root: './fixtures/redirects/',
output: 'server',
adapter: testAdapter(),
redirects: {
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('Astro.redirect', () => {
before(async () => {
process.env.STATIC_MODE = true;
fixture = await loadFixture({
root: './fixtures/ssr-redirect/',
root: './fixtures/redirects/',
output: 'static',
redirects: {
'/old': '/test',
Expand All @@ -78,6 +78,9 @@ describe('Astro.redirect', () => {
status: 302,
destination: '/test',
},
'/more/old/[dynamic]': '/more/[dynamic]',
'/more/old/[dynamic]/[route]': '/more/[dynamic]/[route]',
'/more/old/[...spread]': '/more/new/[...spread]',
},
});
await fixture.build();
Expand Down Expand Up @@ -149,6 +152,12 @@ describe('Astro.redirect', () => {
expect(html).to.include('http-equiv="refresh');
expect(html).to.include('url=/test');
});

it('falls back to spread rule when dynamic rules should not match', async () => {
const html = await fixture.readFile('/more/old/welcome/world/index.html');
expect(html).to.include('http-equiv="refresh');
expect(html).to.include('url=/more/new/welcome/world');
});
});

describe('dev', () => {
Expand All @@ -157,10 +166,13 @@ describe('Astro.redirect', () => {
before(async () => {
process.env.STATIC_MODE = true;
fixture = await loadFixture({
root: './fixtures/ssr-redirect/',
root: './fixtures/redirects/',
output: 'static',
redirects: {
'/one': '/',
'/more/old/[dynamic]': '/more/[dynamic]',
'/more/old/[dynamic]/[route]': '/more/[dynamic]/[route]',
'/more/old/[...spread]': '/more/new/[...spread]',
},
});
devServer = await fixture.startDevServer();
Expand All @@ -170,11 +182,27 @@ describe('Astro.redirect', () => {
await devServer.stop();
});

it('Returns 301', async () => {
it('performs simple redirects', async () => {
let res = await fixture.fetch('/one', {
redirect: 'manual',
});
expect(res.status).to.equal(301);
expect(res.headers.get('Location')).to.equal('/');
});

it('performs dynamic redirects', async () => {
const response = await fixture.fetch('/more/old/hello', { redirect: 'manual' });
expect(response.headers.get('Location')).to.equal('/more/hello');
});

it('performs dynamic redirects with multiple params', async () => {
const response = await fixture.fetch('/more/old/hello/world', { redirect: 'manual' });
expect(response.headers.get('Location')).to.equal('/more/hello/world');
});

it.skip('falls back to spread rule when dynamic rules should not match', async () => {
const response = await fixture.fetch('/more/old/welcome/world', { redirect: 'manual' });
expect(response.headers.get('Location')).to.equal('/more/new/welcome/world');
});
});
});
Expand All @@ -183,7 +211,7 @@ describe('Astro.redirect', () => {
before(async () => {
process.env.STATIC_MODE = true;
fixture = await loadFixture({
root: './fixtures/ssr-redirect/',
root: './fixtures/redirects/',
output: 'static',
redirects: {
'/one': '/',
Expand Down
12 changes: 6 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 408b50c

Please sign in to comment.