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

feat(hybrid): Clean logging and misc tweaks for hybrid removal #11942

Merged
merged 4 commits into from
Sep 9, 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
2 changes: 1 addition & 1 deletion packages/astro/e2e/custom-client-directives.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ test.describe('Custom Client Directives - build server', () => {
adapter: testAdapter({
extendAdapter: {
adapterFeatures: {
forceServerOutput: false,
buildOutput: 'static',
},
},
}),
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export async function generatePages(options: StaticBuildOptions, internals: Buil
// If we don't delete it here, it's technically not impossible (albeit improbable) for it to leak
if (ssr && !hasPrerenderedPages(internals)) {
delete globalThis?.astroAsset?.addStaticImage;
return;
}

const verb = ssr ? 'prerendering' : 'generating';
Expand Down Expand Up @@ -417,7 +416,7 @@ async function generatePath(
url,
headers: new Headers(),
logger,
staticLike: true,
isPrerendered: true,
});
const renderContext = RenderContext.create({ pipeline, pathname, request, routeData: route });

Expand Down
6 changes: 4 additions & 2 deletions packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ class AstroBuilder {
injectImageEndpoint(this.settings, this.manifest, 'build');
}

await runHookConfigDone({ settings: this.settings, logger: logger, command: 'build' });

// If we're building for the server, we need to ensure that an adapter is installed.
// If the adapter installed does not support a server output, an error will be thrown when the adapter is added, so no need to check here.
if (!this.settings.config.adapter && this.settings.buildOutput === 'server') {
throw new AstroError(AstroErrorData.NoAdapterInstalled);
}
Expand All @@ -147,7 +150,6 @@ class AstroBuilder {
manifest: this.manifest,
},
);
await runHookConfigDone({ settings: this.settings, logger: logger });

const { syncInternal } = await import('../sync/index.js');
await syncInternal({
Expand Down Expand Up @@ -239,7 +241,7 @@ class AstroBuilder {
logger: this.logger,
timeStart: this.timer.init,
pageCount: pageNames.length,
buildMode: this.settings.config.output,
buildMode: this.settings.buildOutput!, // buildOutput is always set at this point
});
}
}
Expand Down
5 changes: 2 additions & 3 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { teardown } from '@astrojs/compiler';
import glob from 'fast-glob';
import { bgGreen, bgMagenta, black, green } from 'kleur/colors';
import { bgGreen, black, green } from 'kleur/colors';
import * as vite from 'vite';
import { PROPAGATED_ASSET_FLAG } from '../../content/consts.js';
import {
Expand Down Expand Up @@ -150,12 +150,11 @@ export async function staticBuild(
settings.timer.start('Server generate');
await generatePages(opts, internals);
await cleanStaticOutput(opts, internals);
opts.logger.info(null, `\n${bgMagenta(black(' finalizing server assets '))}\n`);
await ssrMoveAssets(opts);
settings.timer.end('Server generate');
return;
}
default: // `settings.buildOutput` will always be one of the above, but TS doesn't know that
default: // `settings.buildOutput` will always be one of the above at this point, but TS doesn't know that
return;
}
}
Expand Down
50 changes: 50 additions & 0 deletions packages/astro/src/core/dev/adapter-validation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { getAdapterStaticRecommendation } from '../../integrations/features-validation.js';
import type { AstroSettings } from '../../types/astro.js';
import type { AstroAdapter } from '../../types/public/integrations.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import type { Logger } from '../logger/core.js';

let hasWarnedMissingAdapter = false;

export function warnMissingAdapter(logger: Logger, settings: AstroSettings) {
if (hasWarnedMissingAdapter) return;
if (settings.buildOutput === 'server' && !settings.config.adapter) {
logger.warn(
'config',
'This project contains server-rendered routes, but no adapter is installed. This is fine for development, but an adapter will be required to build your site for production.',
);
hasWarnedMissingAdapter = true;
}
}

export function validateSetAdapter(
logger: Logger,
settings: AstroSettings,
adapter: AstroAdapter,
maybeConflictingIntegration: string,
command?: 'dev' | 'build' | string,
) {
if (settings.adapter && settings.adapter.name !== adapter.name) {
throw new Error(
`Integration "${maybeConflictingIntegration}" conflicts with "${settings.adapter.name}". You can only configure one deployment integration.`,
);
}

if (settings.buildOutput === 'server' && adapter.adapterFeatures?.buildOutput === 'static') {
// If the adapter is not compatible with the build output, throw an error
if (command === 'build') {
const adapterRecommendation = getAdapterStaticRecommendation(adapter.name);

throw new AstroError({
...AstroErrorData.AdapterSupportOutputMismatch,
message: AstroErrorData.AdapterSupportOutputMismatch.message(adapter.name),
hint: adapterRecommendation ? adapterRecommendation : undefined,
});
} else if (command === 'dev') {
logger.warn(
null,
`The adapter ${adapter.name} does not support emitting a server output, but the project contain server-rendered pages. Your project will not build correctly.`,
);
}
}
}
6 changes: 5 additions & 1 deletion packages/astro/src/core/dev/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { apply as applyPolyfill } from '../polyfill.js';
import { injectDefaultDevRoutes } from '../routing/dev-default.js';
import { createRouteManifest } from '../routing/index.js';
import { syncInternal } from '../sync/index.js';
import { warnMissingAdapter } from './adapter-validation.js';

export interface Container {
fs: typeof nodeFs;
Expand Down Expand Up @@ -87,6 +88,10 @@ export async function createContainer({

manifest = injectDefaultDevRoutes(settings, devSSRManifest, manifest);

await runHookConfigDone({ settings, logger, command: 'dev' });

warnMissingAdapter(logger, settings);

const viteConfig = await createVite(
{
mode: 'development',
Expand All @@ -107,7 +112,6 @@ export async function createContainer({
},
);

await runHookConfigDone({ settings, logger });
await syncInternal({
settings,
logger,
Expand Down
24 changes: 20 additions & 4 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,8 @@ export const PrerenderClientAddressNotAvailable = {
*/
export const StaticClientAddressNotAvailable = {
Copy link
Member Author

Choose a reason for hiding this comment

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

This message can show in middleware code, I'm not exactly sure why though.

name: 'StaticClientAddressNotAvailable',
title: '`Astro.clientAddress` is not available in static mode.',
// TODO: Update this for the new static mode? I'm not sure this error can even still happen.
message:
"`Astro.clientAddress` is only available when using `output: 'server'` or `output: 'hybrid'`. Update your Astro config if you need SSR features.",
title: '`Astro.clientAddress` is not available in prerendered pages.',
message: '`Astro.clientAddress` is only available on pages that are server-rendered.',
hint: 'See https://docs.astro.build/en/guides/server-side-rendering/ for more information on how to enable SSR.',
} satisfies ErrorData;
/**
Expand Down Expand Up @@ -396,6 +394,24 @@ export const NoAdapterInstalled = {
message: `Cannot use server-rendered pages without an adapter. Please install and configure the appropriate server adapter for your final deployment.`,
hint: 'See https://docs.astro.build/en/guides/server-side-rendering/ for more information.',
} satisfies ErrorData;

/**
* @docs
* @see
* - [Server-side Rendering](https://docs.astro.build/en/guides/server-side-rendering/)
* @description
* The currently configured adapter does not support server-side rendering, which is required for the current project setup.
*
* Depending on your adapter, there may be a different entrypoint to use for server-side rendering. For example, the `@astrojs/vercel` adapter has a `@astrojs/vercel/static` entrypoint for static rendering, and a `@astrojs/vercel/serverless` entrypoint for server-side rendering.
*
*/
export const AdapterSupportOutputMismatch = {
name: 'AdapterSupportOutputMismatch',
title: 'Adapter does not support server output.',
message: (adapterName: string) =>
`The \`${adapterName}\` adapter is configured to output a static website, but the project contains server-rendered pages. Please install and configure the appropriate server adapter for your final deployment.`,
} satisfies ErrorData;

/**
* @docs
* @description
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/preview/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default async function preview(inlineConfig: AstroInlineConfig): Promise<
// Create a route manifest so we can know if the build output is a static site or not
await createRouteManifest({ settings: settings, cwd: inlineConfig.root }, logger);

await runHookConfigDone({ settings: settings, logger: logger });
await runHookConfigDone({ settings: settings, logger: logger, command: 'preview' });

if (settings.buildOutput === 'static') {
if (!fs.existsSync(settings.config.outDir)) {
Expand Down
18 changes: 8 additions & 10 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,17 +480,15 @@ export class RenderContext {
return Reflect.get(request, clientAddressSymbol) as string;
}

if (pipeline.serverLike) {
if (request.body === null) {
throw new AstroError(AstroErrorData.PrerenderClientAddressNotAvailable);
}
if (request.body === null) {
throw new AstroError(AstroErrorData.PrerenderClientAddressNotAvailable);
}

if (pipeline.adapterName) {
throw new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message(pipeline.adapterName),
});
}
if (pipeline.adapterName) {
throw new AstroError({
...AstroErrorData.ClientAddressNotAvailable,
message: AstroErrorData.ClientAddressNotAvailable.message(pipeline.adapterName),
});
}

throw new AstroError(AstroErrorData.StaticClientAddressNotAvailable);
Expand Down
17 changes: 9 additions & 8 deletions packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export interface CreateRequestOptions {
*
* @default false
*/
staticLike?: boolean;
isPrerendered?: boolean;
}

const clientAddressSymbol = Symbol.for('astro.clientAddress');
Expand All @@ -41,10 +41,10 @@ export function createRequest({
body = undefined,
logger,
locals,
staticLike = false,
isPrerendered = false,
}: CreateRequestOptions): Request {
// 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
const headersObj = isPrerendered
? undefined
: headers instanceof Headers
? headers
Expand All @@ -58,27 +58,28 @@ export function createRequest({

if (typeof url === 'string') url = new URL(url);

if (staticLike) {
// Remove search parameters if the request is for a page that will be on-demand rendered
if (isPrerendered) {
url.search = '';
}

const request = new Request(url, {
method: method,
headers: headersObj,
// body is made available only if the request is for a page that will be on-demand rendered
body: staticLike ? null : body,
body: isPrerendered ? null : body,
});

if (staticLike) {
// Warn when accessing headers in SSG mode
if (isPrerendered) {
// Warn when accessing headers in prerendered pages
const _headers = request.headers;
const headersDesc = Object.getOwnPropertyDescriptor(request, 'headers') || {};
Object.defineProperty(request, 'headers', {
...headersDesc,
get() {
logger.warn(
null,
`\`Astro.request.headers\` is unavailable in "static" output mode, and in prerendered pages within "hybrid" and "server" output modes. If you need access to request headers, make sure that \`output\` is configured as either \`"server"\` or \`output: "hybrid"\` in your config file, and that the page accessing the headers is rendered on-demand.`,
`\`Astro.request.headers\` is not available on prerendered pages. If you need access to request headers, make sure that the page is server rendered using \`export const prerender = false;\` or by setting \`output\` to \`"server"\` in your Astro config to make all your pages server rendered.`,
);
return _headers;
},
Expand Down
42 changes: 11 additions & 31 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { fileURLToPath } from 'node:url';
import { bold } from 'kleur/colors';
import pLimit from 'p-limit';
import { toRoutingStrategy } from '../../../i18n/utils.js';
import { runHookRouteSetup } from '../../../integrations/hooks.js';
import { getPrerenderDefault } from '../../../prerender/utils.js';
import type { AstroConfig } from '../../../types/public/config.js';
import type { RouteData, RoutePart } from '../../../types/public/internal.js';
Expand All @@ -20,6 +19,7 @@ import { resolvePages } from '../../util.js';
import { routeComparator } from '../priority.js';
import { getRouteGenerator } from './generator.js';
import { getPattern } from './pattern.js';
import { getRoutePrerenderOption } from './prerender.js';
const require = createRequire(import.meta.url);

interface Item {
Expand Down Expand Up @@ -506,7 +506,16 @@ export async function createRouteManifest(
let promises = [];
for (const route of routes) {
promises.push(
limit(async () => await getRoutePrerenderOption(route, settings, params.fsMod, logger)),
limit(async () => {
if (route.type !== 'page' && route.type !== 'endpoint') return;
const localFs = params.fsMod ?? nodeFs;
const content = await localFs.promises.readFile(
fileURLToPath(new URL(route.component, settings.config.root)),
'utf-8',
);

await getRoutePrerenderOption(content, route, settings, logger);
}),
);
}
await Promise.all(promises);
Expand Down Expand Up @@ -714,35 +723,6 @@ export async function createRouteManifest(
};
}

async function getRoutePrerenderOption(
route: RouteData,
settings: AstroSettings,
fsMod: typeof nodeFs | undefined,
logger: Logger,
) {
if (route.type !== 'page' && route.type !== 'endpoint') return;
const localFs = fsMod ?? nodeFs;
const content = await localFs.promises.readFile(
fileURLToPath(new URL(route.component, settings.config.root)),
'utf-8',
);

// Check if the route is pre-rendered or not
const match = /^\s*export\s+const\s+prerender\s*=\s*(true|false);?/m.exec(content);
if (match) {
route.prerender = match[1] === 'true';
}

await runHookRouteSetup({ route, settings, logger });

// If not explicitly set, default to the global setting
if (typeof route.prerender === undefined) {
route.prerender = getPrerenderDefault(settings.config);
}

if (!route.prerender) settings.buildOutput = 'server';
}

export function resolveInjectedRoute(entrypoint: string, root: URL, cwd?: string) {
let resolved;
try {
Expand Down
29 changes: 29 additions & 0 deletions packages/astro/src/core/routing/manifest/prerender.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { runHookRouteSetup } from '../../../integrations/hooks.js';
import { getPrerenderDefault } from '../../../prerender/utils.js';
import type { AstroSettings } from '../../../types/astro.js';
import type { RouteData } from '../../../types/public/internal.js';
import type { Logger } from '../../logger/core.js';

const PRERENDER_REGEX = /^\s*export\s+const\s+prerender\s*=\s*(true|false);?/m;

export async function getRoutePrerenderOption(
content: string,
route: RouteData,
settings: AstroSettings,
logger: Logger,
) {
// Check if the route is pre-rendered or not
const match = PRERENDER_REGEX.exec(content);
if (match) {
route.prerender = match[1] === 'true';
}

await runHookRouteSetup({ route, settings, logger });

// If not explicitly set, default to the global setting
if (typeof route.prerender === undefined) {
route.prerender = getPrerenderDefault(settings.config);
}

if (!route.prerender) settings.buildOutput = 'server';
}
2 changes: 1 addition & 1 deletion packages/astro/src/core/sync/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default async function sync(
logger,
});
const manifest = await createRouteManifest({ settings, fsMod: fs }, logger);
await runHookConfigDone({ settings, logger });
await runHookConfigDone({ settings, logger, command: 'sync' });
return await syncInternal({ settings, logger, fs, force: inlineConfig.force, manifest });
}

Expand Down
Loading
Loading