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): cosider base when special-casing /_image #10274

Merged
merged 5 commits into from
Feb 29, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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