From e556151603a2f0173059d0f98fdcbec0610b48ff Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Fri, 1 Mar 2024 00:08:01 +0530 Subject: [PATCH] fix(dev): cosider `base` when special-casing `/_image` (#10274) * fix(dev): cosider `base` when special-casing `/_image` * add changeset * adjust tests * Apply suggestions from code review * add test --- .changeset/curvy-donkeys-knock.md | 5 ++++ .../src/pages/form-three.astro | 1 + packages/astro/src/core/build/generate.ts | 5 ++-- packages/astro/src/core/request.ts | 30 ++++++++++++++----- .../src/vite-plugin-astro-server/route.ts | 18 +++++------ packages/astro/test/core-image.test.js | 11 +++++++ packages/astro/test/test-adapter.js | 4 ++- 7 files changed, 53 insertions(+), 21 deletions(-) create mode 100644 .changeset/curvy-donkeys-knock.md diff --git a/.changeset/curvy-donkeys-knock.md b/.changeset/curvy-donkeys-knock.md new file mode 100644 index 000000000000..0197a800e052 --- /dev/null +++ b/.changeset/curvy-donkeys-knock.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a regression introduced in v4.4.5 where image optimization did not work in dev mode when a base was configured. diff --git a/packages/astro/e2e/fixtures/view-transitions/src/pages/form-three.astro b/packages/astro/e2e/fixtures/view-transitions/src/pages/form-three.astro index 9d27534de8c1..a77a63103842 100644 --- a/packages/astro/e2e/fixtures/view-transitions/src/pages/form-three.astro +++ b/packages/astro/e2e/fixtures/view-transitions/src/pages/form-three.astro @@ -4,6 +4,7 @@ let formData: FormData | undefined; if(Astro.request.method === 'POST') { formData = await Astro.request.formData(); } +export const prerender = false --- { diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index fea2447b0ba3..0b8594bb9618 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -471,7 +471,7 @@ async function generatePath( route: RouteData ) { const { mod } = gopts; - const { config, logger, options, serverLike } = pipeline; + const { config, logger, options } = pipeline; logger.debug('build', `Generating: ${pathname}`); // This adds the page name to the array so it can be shown as part of stats. @@ -502,10 +502,11 @@ async function generatePath( ); const request = createRequest({ + base: config.base, url, headers: new Headers(), logger, - ssr: serverLike, + staticLike: true, }); const renderContext = RenderContext.create({ pipeline, pathname, request, routeData: route }); diff --git a/packages/astro/src/core/request.ts b/packages/astro/src/core/request.ts index 4f70c99dc14b..13b9de4a5995 100644 --- a/packages/astro/src/core/request.ts +++ b/packages/astro/src/core/request.ts @@ -1,54 +1,67 @@ import type { IncomingHttpHeaders } from 'node:http'; import type { Logger } from './logger/core.js'; +import { appendForwardSlash, prependForwardSlash } from './path.js'; type HeaderType = Headers | Record | IncomingHttpHeaders; type RequestBody = ArrayBuffer | Blob | ReadableStream | URLSearchParams | FormData; export interface CreateRequestOptions { + base: string; url: URL | string; clientAddress?: string | undefined; headers: HeaderType; method?: string; body?: RequestBody | undefined; logger: Logger; - ssr: boolean; locals?: object | undefined; - removeParams?: boolean; + /** + * Whether the request is being created for a static build or for a prerendered page within a hybrid/SSR build, or for emulating one of those in dev mode. + * + * When `true`, the request will not include search parameters or body, and warn when headers are accessed. + * + * @default false + */ + staticLike?: boolean; } const clientAddressSymbol = Symbol.for('astro.clientAddress'); const clientLocalsSymbol = Symbol.for('astro.locals'); export function createRequest({ + base, url, headers, clientAddress, method = 'GET', body = undefined, logger, - ssr, locals, - removeParams = false, + staticLike = false }: CreateRequestOptions): Request { - let headersObj = + // headers are made available on the created request only if the request is for a page that will be on-demand rendered + const headersObj = + staticLike ? undefined : headers instanceof Headers ? headers : new Headers(Object.entries(headers as Record)); if (typeof url === 'string') url = new URL(url); + const imageEndpoint = prependForwardSlash(appendForwardSlash(base)) + '_image'; + // HACK! astro:assets uses query params for the injected route in `dev` - if (removeParams && url.pathname !== '/_image') { + if (staticLike && url.pathname !== imageEndpoint) { url.search = ''; } const request = new Request(url, { method: method, headers: headersObj, - body, + // body is made available only if the request is for a page that will be on-demand rendered + body: staticLike ? null : body, }); - if (!ssr) { + if (staticLike) { // Warn when accessing headers in SSG mode const _headers = request.headers; const headersDesc = Object.getOwnPropertyDescriptor(request, 'headers') || {}; @@ -63,6 +76,7 @@ export function createRequest({ }, }); } else if (clientAddress) { + // clientAddress is stored to be read by RenderContext, only if the request is for a page that will be on-demand rendered Reflect.set(request, clientAddressSymbol, clientAddress); } diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index be904d5eb965..8a737c2af097 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -10,7 +10,6 @@ import { createRequest } from '../core/request.js'; import { matchAllRoutes } from '../core/routing/index.js'; import { normalizeTheLocale } from '../i18n/index.js'; import { getSortedPreloadedMatches } from '../prerender/routing.js'; -import { isServerLikeOutput } from '../prerender/utils.js'; import type { DevPipeline } from './pipeline.js'; import { handle404Response, writeSSRResult, writeWebResponse } from './response.js'; @@ -146,8 +145,6 @@ export async function handleRoute({ return handle404Response(origin, incomingRequest, incomingResponse); } - const buildingToSSR = isServerLikeOutput(config); - let request: Request; let renderContext: RenderContext; let mod: ComponentInstance | undefined = undefined; @@ -183,10 +180,12 @@ export async function handleRoute({ return handle404Response(origin, incomingRequest, incomingResponse); } request = createRequest({ + base: config.base, url, - headers: buildingToSSR ? incomingRequest.headers : new Headers(), + headers: incomingRequest.headers, logger, - ssr: buildingToSSR, + // no route found, so we assume the default for rendering the 404 page + staticLike: config.output === "static" || config.output === "hybrid", }); route = { component: '', @@ -221,15 +220,14 @@ export async function handleRoute({ // Allows adapters to pass in locals in dev mode. const locals = Reflect.get(incomingRequest, clientLocalsSymbol); request = createRequest({ + base: config.base, url, - // Headers are only available when using SSR. - headers: buildingToSSR ? incomingRequest.headers : new Headers(), + headers: incomingRequest.headers, method: incomingRequest.method, body, logger, - ssr: buildingToSSR, - clientAddress: buildingToSSR ? incomingRequest.socket.remoteAddress : undefined, - removeParams: buildingToSSR === false || route.prerender, + clientAddress: incomingRequest.socket.remoteAddress, + staticLike: config.output === "static" || route.prerender, }); // Set user specified headers to response object. diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index 8724df88bac0..3b240e606b0b 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -1041,12 +1041,14 @@ describe('astro:image', () => { }); describe('dev ssr', () => { + /** @type {import('./test-utils').DevServer} */ let devServer; before(async () => { fixture = await loadFixture({ root: './fixtures/core-image-ssr/', output: 'server', adapter: testAdapter(), + base: 'some-base', image: { service: testImageService(), }, @@ -1058,6 +1060,15 @@ describe('astro:image', () => { await devServer.stop(); }); + it('serves the image at /_image', async () => { + const params = new URLSearchParams; + params.set("href", "/src/assets/penguin1.jpg?origWidth=207&origHeight=243&origFormat=jpg"); + params.set("f", "webp"); + const response = await fixture.fetch("/some-base/_image?" + String(params)); + assert.equal(response.status, 200); + assert.equal(response.headers.get('content-type'), 'image/webp'); + }); + it('does not interfere with query params', async () => { let res = await fixture.fetch('/api?src=image.png'); const html = await res.text(); diff --git a/packages/astro/test/test-adapter.js b/packages/astro/test/test-adapter.js index 0a30583b22e7..ba8e7a0c8fec 100644 --- a/packages/astro/test/test-adapter.js +++ b/packages/astro/test/test-adapter.js @@ -77,7 +77,9 @@ export default function ( name: 'my-ssr-adapter', serverEntrypoint: '@my-ssr', exports: ['manifest', 'createApp'], - supportedAstroFeatures: {}, + supportedAstroFeatures: { + serverOutput: "stable" + }, ...extendAdapter, }); },