Skip to content

Commit

Permalink
fix(redirects): handle non-verbatim targets (#9089)
Browse files Browse the repository at this point in the history
* add tests

* implement fix

* add changeset

* `for const in` -> `for const of`

* reskip: external redirects are still not ignored
  • Loading branch information
lilnasy authored Dec 19, 2023
1 parent 25e6670 commit 5ae6578
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/silly-cycles-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where redirects did not replace slugs when the target of the redirect rule was not a verbatim route in the project.
9 changes: 8 additions & 1 deletion packages/astro/src/core/redirects/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,14 @@ export function redirectRouteGenerate(redirectRoute: RouteData, data: Params): s
if (typeof routeData !== 'undefined') {
return routeData?.generate(data) || routeData?.pathname || '/';
} else if (typeof route === 'string') {
return route;
// TODO: this logic is duplicated between here and manifest/create.ts
let target = route
for (const param of Object.keys(data)) {
const paramValue = data[param]!
target = target.replace(`[${param}]`, paramValue)
target = target.replace(`[...${param}]`, paramValue)
}
return target
} else if (typeof route === 'undefined') {
return '/';
}
Expand Down
28 changes: 28 additions & 0 deletions packages/astro/test/redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ describe('Astro.redirect', () => {
redirects: {
'/api/redirect': '/test',
'/external/redirect': 'https://example.com/',
// for example, the real file handling the target path may be called
// src/pages/not-verbatim/target1/[something-other-than-dynamic].astro
'/source/[dynamic]': '/not-verbatim/target1/[dynamic]',
// may be called src/pages/not-verbatim/target2/[abc]/[xyz].astro
'/source/[dynamic]/[route]': '/not-verbatim/target2/[dynamic]/[route]',
// may be called src/pages/not-verbatim/target3/[...rest].astro
'/source/[...spread]': '/not-verbatim/target3/[...spread]',
},
});
await fixture.build();
Expand Down Expand Up @@ -68,6 +75,27 @@ describe('Astro.redirect', () => {
const response = await app.render(request);
expect(response.status).to.equal(308);
});

it('Forwards params to the target path - single param', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/source/x');
const response = await app.render(request);
expect(response.headers.get("Location")).to.equal("/not-verbatim/target1/x");
});

it('Forwards params to the target path - multiple params', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/source/x/y');
const response = await app.render(request);
expect(response.headers.get("Location")).to.equal("/not-verbatim/target2/x/y");
});

it('Forwards params to the target path - spread param', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/source/x/y/z');
const response = await app.render(request);
expect(response.headers.get("Location")).to.equal("/not-verbatim/target3/x/y/z");
});
});
});

Expand Down

0 comments on commit 5ae6578

Please sign in to comment.