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(dev): remove params for prerendered pages #10199

Merged
merged 6 commits into from
Feb 23, 2024
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/nice-hats-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes an issue where prerendered pages had access to query params in dev mode.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { APIContext } from 'astro';

export const prerender = false;

export const POST = async ({ request, redirect }: APIContext) => {
const formData = await request.formData();
const name = formData.get('name');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Layout from '../components/Layout.astro';
const method = Astro.url.searchParams.get('method') ?? 'POST';
const enctype = Astro.url.searchParams.get('enctype');
const postShowThrow = Astro.url.searchParams.has('throw') ?? false;
export const prerender = false;
---

<Layout>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
---
import Layout from '../components/Layout.astro';
const name = Astro.url.searchParams.get('name');

export const prerender = false;
---
<Layout>
<div>Submitted contact: <span id="contact-name">{name}</span></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ if(Astro.request.method === 'POST') {
const name = formData.get('name');
return Astro.redirect(`/form-response?name=${name}`);
}

export const prerender = false;
---
<Layout>
<h2>Contact Form</h2>
Expand Down
11 changes: 10 additions & 1 deletion packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface CreateRequestOptions {
logger: Logger;
ssr: boolean;
locals?: object | undefined;
removeParams?: boolean;
}

const clientAddressSymbol = Symbol.for('astro.clientAddress');
Expand All @@ -27,13 +28,21 @@ export function createRequest({
logger,
ssr,
locals,
removeParams = false,
}: CreateRequestOptions): Request {
let headersObj =
headers instanceof Headers
? headers
: new Headers(Object.entries(headers as Record<string, any>));

const request = new Request(url.toString(), {
if (typeof url === 'string') url = new URL(url);

// HACK! astro:assets uses query params for the injected route in `dev`
if (removeParams && url.pathname !== '/_image') {
url.search = '';
}
Comment on lines +40 to +43
Copy link
Member

Choose a reason for hiding this comment

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

😔 I know it's not new code, but still hate it, ha.


const request = new Request(url, {
method: method,
headers: headersObj,
body,
Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ export async function handleRequest({
// Add config.base back to url before passing it to SSR
url.pathname = removeTrailingForwardSlash(config.base) + url.pathname;

// HACK! astro:assets uses query params for the injected route in `dev`
if (!buildingToSSR && pathname !== '/_image') {
// Prevent user from depending on search params when not doing SSR.
// NOTE: Create an array copy here because deleting-while-iterating
// creates bugs where not all search params are removed.
const allSearchParams = Array.from(url.searchParams);
for (const [key] of allSearchParams) {
url.searchParams.delete(key);
}
}

let body: ArrayBuffer | undefined = undefined;
if (!(incomingRequest.method === 'GET' || incomingRequest.method === 'HEAD')) {
let bytes: Uint8Array[] = [];
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ export async function handleRoute({
logger,
ssr: buildingToSSR,
clientAddress: buildingToSSR ? incomingRequest.socket.remoteAddress : undefined,
removeParams: buildingToSSR === false || route.prerender
});

// Set user specified headers to response object.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import * as assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { after, before, describe, it } from 'node:test';
import { fileURLToPath } from 'node:url';
import { createContainer } from '../../../dist/core/dev/container.js';
import { createLoader } from '../../../dist/core/module-loader/index.js';
import { createRouteManifest } from '../../../dist/core/routing/index.js';
import { createComponent, render } from '../../../dist/runtime/server/index.js';
import { createController, handleRequest } from '../../../dist/vite-plugin-astro-server/index.js';
import { DevPipeline } from '../../../dist/vite-plugin-astro-server/pipeline.js';
import { createDevelopmentManifest } from '../../../dist/vite-plugin-astro-server/plugin.js';
import testAdapter from '../../test-adapter.js';
import {
createAstroModule,
createBasicSettings,
Expand Down Expand Up @@ -73,4 +76,55 @@ describe('vite-plugin-astro-server', () => {
assert.equal(html.includes('<div id="test">'), true);
});
});

describe('url', () => {
let container;
let settings;

before(async () => {
const root = new URL('../../fixtures/api-routes/', import.meta.url);
const fileSystem = {
'/src/pages/url.astro': `{Astro.request.url}`,
'/src/pages/prerendered.astro': `---
export const prerender = true;
---
{Astro.request.url}`,
};
const fs = createFs(fileSystem, root);
settings = await createBasicSettings({
root: fileURLToPath(root),
output: 'server',
adapter: testAdapter(),
});
container = await createContainer({
fs,
settings,
logger: defaultLogger,
});
});

after(async () => {
await container.close();
});

it('params are included', async () => {
const { req, res, text } = createRequestAndResponse({
method: 'GET',
url: '/url?xyz=123',
});
container.handle(req, res);
const html = await text();
assert.deepEqual(html, '<!DOCTYPE html>http://undefined/url?xyz=123');
});

it('params are excluded on prerendered routes', async () => {
const { req, res, text } = createRequestAndResponse({
method: 'GET',
url: '/prerendered?xyz=123',
});
container.handle(req, res);
const html = await text();
assert.deepEqual(html, '<!DOCTYPE html>http://undefined/prerendered');
});
});
});
Loading