Skip to content

Commit

Permalink
fix(dev): cosider base when special-casing /_image (#10274)
Browse files Browse the repository at this point in the history
* fix(dev): cosider `base` when special-casing `/_image`

* add changeset

* adjust tests

* Apply suggestions from code review

* add test
  • Loading branch information
lilnasy authored Feb 29, 2024
1 parent 3757a21 commit e556151
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/curvy-donkeys-knock.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ let formData: FormData | undefined;
if(Astro.request.method === 'POST') {
formData = await Astro.request.formData();
}
export const prerender = false
---
<Layout>
{
Expand Down
5 changes: 3 additions & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 });

Expand Down
30 changes: 22 additions & 8 deletions packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
@@ -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<string, any> | 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<string, any>));

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') || {};
Expand All @@ -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);
}

Expand Down
18 changes: 8 additions & 10 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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: '',
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions packages/astro/test/core-image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
Expand All @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/test/test-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ export default function (
name: 'my-ssr-adapter',
serverEntrypoint: '@my-ssr',
exports: ['manifest', 'createApp'],
supportedAstroFeatures: {},
supportedAstroFeatures: {
serverOutput: "stable"
},
...extendAdapter,
});
},
Expand Down

0 comments on commit e556151

Please sign in to comment.