Skip to content

Commit

Permalink
Fix double prepended forward slash in certain cases (#7091)
Browse files Browse the repository at this point in the history
* test: add test with no base

* fix: don't always prepend a forward slash

* chore: changeset

* `'/' + base`   ------> `prependForwardSlash(base)`
  • Loading branch information
MoustaphaDev committed May 15, 2023
1 parent f994ebd commit cd410c5
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 46 deletions.
5 changes: 5 additions & 0 deletions .changeset/swift-moons-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix double prepended forward slash in SSR
6 changes: 3 additions & 3 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { call as callEndpoint, createAPIContext } from '../endpoint/index.js';
import { consoleLogDestination } from '../logger/console.js';
import { error, type LogOptions } from '../logger/core.js';
import { callMiddleware } from '../middleware/callMiddleware.js';
import { removeTrailingForwardSlash } from '../path.js';
import { prependForwardSlash, removeTrailingForwardSlash } from '../path.js';
import {
createEnvironment,
createRenderContext,
Expand Down Expand Up @@ -101,7 +101,7 @@ export class App {
if (this.#manifest.assets.has(url.pathname)) {
return undefined;
}
let pathname = '/' + this.removeBase(url.pathname);
let pathname = prependForwardSlash(this.removeBase(url.pathname));
let routeData = matchRoute(pathname, this.#manifestData);

if (routeData) {
Expand Down Expand Up @@ -178,7 +178,7 @@ export class App {
status = 200
): Promise<Response> {
const url = new URL(request.url);
const pathname = '/' + this.removeBase(url.pathname);
const pathname = prependForwardSlash(this.removeBase(url.pathname));
const info = this.#routeDataToRouteInfo.get(routeData!)!;
// may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc.
const links = new Set<never>();
Expand Down
133 changes: 90 additions & 43 deletions packages/integrations/node/test/prerender.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,61 +9,108 @@ describe('Prerendering', () => {
let fixture;
let server;

before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

after(async () => {
await server.stop();
});

async function load() {
const mod = await import('./fixtures/prerender/dist/server/entry.mjs');
return mod;
}

it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one`);
const html = await res.text();
const $ = cheerio.load(html);
describe('With base', () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('One');
});
after(async () => {
await server.stop();
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`);
const html = await res.text();
const $ = cheerio.load(html);
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('One');
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);
it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Omitting the trailing slash results in a redirect that includes the base', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`, {
redirect: 'manual',
});
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal('/some-base/two/');
});
});
describe('Without base', () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

after(async () => {
await server.stop();
});

it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/one`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('One');
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/two`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);

it('Omitting the trailing slash results in a redirect that includes the base', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`, {
redirect: 'manual',
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal('/some-base/two/');
});
});

0 comments on commit cd410c5

Please sign in to comment.