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

fix(redirects): attempt to get only params in dev mode #8647

Merged
merged 4 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
Comment on lines +203 to 206
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll let someone else tackle this.

});
});
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.