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

refactor: build pipeline #8088

Merged
merged 8 commits into from
Aug 16, 2023
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
1 change: 1 addition & 0 deletions packages/astro/src/cli/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ async function tryToInstallIntegrations({
} catch (err) {
spinner.fail();
debug('add', 'Error installing dependencies', err);
// eslint-disable-next-line no-console
console.error('\n', (err as any).stdout, '\n');
return UpdateResult.failure;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/app/ssrPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class EndpointNotFoundError extends Error {
}

export class SSRRoutePipeline extends Pipeline {
encoder = new TextEncoder();
#encoder = new TextEncoder();

constructor(env: Environment) {
super(env);
Expand All @@ -40,7 +40,7 @@ export class SSRRoutePipeline extends Pipeline {
headers.set('Content-Type', 'text/plain;charset=utf-8');
}
const bytes =
response.encoding !== 'binary' ? this.encoder.encode(response.body) : response.body;
response.encoding !== 'binary' ? this.#encoder.encode(response.body) : response.body;
headers.set('Content-Length', bytes.byteLength.toString());

const newResponse = new Response(bytes, {
Expand Down
211 changes: 211 additions & 0 deletions packages/astro/src/core/build/buildPipeline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
import { Pipeline } from '../pipeline.js';
import type { BuildInternals } from './internal';
import type { PageBuildData, StaticBuildOptions } from './types';
import { ASTRO_PAGE_RESOLVED_MODULE_ID } from './plugins/plugin-pages.js';
import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js';
import { ASTRO_PAGE_EXTENSION_POST_PATTERN } from './plugins/util.js';
import type { SSRManifest } from '../app/types';
import type { AstroConfig, AstroSettings, RouteType, SSRLoadedRenderer } from '../../@types/astro';
import { getOutputDirectory, isServerLikeOutput } from '../../prerender/utils.js';
import type { EndpointCallResult } from '../endpoint';
import { createEnvironment } from '../render/index.js';
import { BEFORE_HYDRATION_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import { createAssetLink } from '../render/ssr-element.js';
import type { BufferEncoding } from 'vfile';

/**
* This pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files.
*/
export class BuildPipeline extends Pipeline {
#internals: BuildInternals;
#staticBuildOptions: StaticBuildOptions;
#manifest: SSRManifest;
#currentEndpointBody?: {
body: string | Uint8Array;
encoding: BufferEncoding;
};

constructor(
staticBuildOptions: StaticBuildOptions,
internals: BuildInternals,
manifest: SSRManifest
) {
const ssr = isServerLikeOutput(staticBuildOptions.settings.config);
super(
createEnvironment({
adapterName: manifest.adapterName,
logging: staticBuildOptions.logging,
mode: staticBuildOptions.mode,
renderers: manifest.renderers,
clientDirectives: manifest.clientDirectives,
compressHTML: manifest.compressHTML,
async resolve(specifier: string) {
const hashedFilePath = manifest.entryModules[specifier];
if (typeof hashedFilePath !== 'string') {
// If no "astro:scripts/before-hydration.js" script exists in the build,
// then we can assume that no before-hydration scripts are needed.
if (specifier === BEFORE_HYDRATION_SCRIPT_ID) {
Copy link

@BlankParticle BlankParticle Sep 2, 2023

Choose a reason for hiding this comment

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

I think this check is the reason that so many hybrid render builds are failing,
in packages/src/core/build/plugins/plugin-manifest.ts:238 you are setting entryModules[BEFORE_HYDRATION_SCRIPT_ID] = '', then here you are checking if (typeof hashedFilePath !== 'string') and inside that check you are checking if (specifier === BEFORE_HYDRATION_SCRIPT_ID), but as you have already set the BEFORE_HYDRATION_SCRIPT_ID in plugin-manifest.ts so this check is never called.

as a result, while resolving BEFORE_HYDRATION_SCRIPT_ID in packages/astro/src/runtime/server/hydration.ts:157 we get "/" instead of "", for which this check passes and before-hydration-url is generated.

if (beforeHydrationUrl.length) {
		island.props['before-hydration-url'] = beforeHydrationUrl;
	}

So the potential fixes could be to change the function to be

async resolve(specifier: string) {
					//const hashedFilePath = manifest.entryModules[specifier];
        // EDIT: This one seems to work, all tests also passes
        const hashedFilePath = internals.entrySpecifierToBundleMap.get(specifier);
					if (typeof hashedFilePath !== 'string') {
					// If no "astro:scripts/before-hydration.js" script exists in the build,
					// then we can assume that no before-hydration scripts are needed.
					if (specifier === BEFORE_HYDRATION_SCRIPT_ID && hashedFilePath === "") {
						return '';
					}
						throw new Error(`Cannot find the built path for ${specifier}`);
					}
					return createAssetLink(hashedFilePath, manifest.base, manifest.assetsPrefix);
				},

or change the check at packages/astro/src/runtime/server/hydration.ts:157 to be

// Its more of a workaround than a fix
if (beforeHydrationUrl.length !== 0 && beforeHydrationUrl !== "/") {
		island.props['before-hydration-url'] = beforeHydrationUrl;
	}

I am down to make a PR to fix the issue (#8379 and many more), just waiting for confrimations from the maintainers

return '';
}
throw new Error(`Cannot find the built path for ${specifier}`);
}
return createAssetLink(hashedFilePath, manifest.base, manifest.assetsPrefix);
},
routeCache: staticBuildOptions.routeCache,
site: manifest.site,
ssr,
streaming: true,
})
);
this.#internals = internals;
this.#staticBuildOptions = staticBuildOptions;
this.#manifest = manifest;
this.setEndpointHandler(this.#handleEndpointResult);
}

getInternals(): Readonly<BuildInternals> {
return this.#internals;
}

getSettings(): Readonly<AstroSettings> {
return this.#staticBuildOptions.settings;
}

getStaticBuildOptions(): Readonly<StaticBuildOptions> {
return this.#staticBuildOptions;
}

getConfig(): AstroConfig {
return this.#staticBuildOptions.settings.config;
}

getManifest(): SSRManifest {
return this.#manifest;
}

/**
* The SSR build emits two important files:
* - dist/server/manifest.mjs
* - dist/renderers.mjs
*
* These two files, put together, will be used to generate the pages.
*
* ## Errors
*
* It will throw errors if the previous files can't be found in the file system.
*
* @param staticBuildOptions
*/
static async retrieveManifest(
staticBuildOptions: StaticBuildOptions,
internals: BuildInternals
): Promise<SSRManifest> {
const config = staticBuildOptions.settings.config;
const baseDirectory = getOutputDirectory(config);
const manifestEntryUrl = new URL(
`${internals.manifestFileName}?time=${Date.now()}`,
baseDirectory
);
const { manifest } = await import(manifestEntryUrl.toString());
if (!manifest) {
throw new Error(
"Astro couldn't find the emitted manifest. This is an internal error, please file an issue."
);
}

const renderersEntryUrl = new URL(`renderers.mjs?time=${Date.now()}`, baseDirectory);
const renderers = await import(renderersEntryUrl.toString());
if (!renderers) {
throw new Error(
"Astro couldn't find the emitted renderers. This is an internal error, please file an issue."
);
}
return {
...manifest,
renderers: renderers.renderers as SSRLoadedRenderer[],
};
}

/**
* It collects the routes to generate during the build.
*
* It returns a map of page information and their relative entry point as a string.
*/
retrieveRoutesToGenerate(): Map<PageBuildData, string> {
const pages = new Map<PageBuildData, string>();

for (const [entryPoint, filePath] of this.#internals.entrySpecifierToBundleMap) {
// virtual pages can be emitted with different prefixes:
// - the classic way are pages emitted with prefix ASTRO_PAGE_RESOLVED_MODULE_ID -> plugin-pages
// - pages emitted using `build.split`, in this case pages are emitted with prefix RESOLVED_SPLIT_MODULE_ID
if (
entryPoint.includes(ASTRO_PAGE_RESOLVED_MODULE_ID) ||
entryPoint.includes(RESOLVED_SPLIT_MODULE_ID)
) {
const [, pageName] = entryPoint.split(':');
const pageData = this.#internals.pagesByComponent.get(
`${pageName.replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}`
);
if (!pageData) {
throw new Error(
"Build failed. Astro couldn't find the emitted page from " + pageName + ' pattern'
);
}

pages.set(pageData, filePath);
}
}
for (const [path, pageData] of this.#internals.pagesByComponent.entries()) {
if (pageData.route.type === 'redirect') {
pages.set(pageData, path);
}
}
return pages;
}

async #handleEndpointResult(request: Request, response: EndpointCallResult): Promise<Response> {
if (response.type === 'response') {
if (!response.response.body) {
return new Response(null);
}
const ab = await response.response.arrayBuffer();
const body = new Uint8Array(ab);
this.#currentEndpointBody = {
body: body,
encoding: 'utf-8',
};
return response.response;
} else {
if (response.encoding) {
this.#currentEndpointBody = {
body: response.body,
encoding: response.encoding,
};
const headers = new Headers();
headers.set('X-Astro-Encoding', response.encoding);
return new Response(response.body, {
headers,
});
Comment on lines +184 to +188
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought we could store the encoding coming from a simple response using an internal header. What do you think? When generating a page, we read it and use it when we write the file to file system.

Copy link
Member

Choose a reason for hiding this comment

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

I like it!

} else {
return new Response(response.body);
}
}
}

async computeBodyAndEncoding(
routeType: RouteType,
response: Response
): Promise<{
body: string | Uint8Array;
encoding: BufferEncoding;
}> {
const encoding = response.headers.get('X-Astro-Encoding') ?? 'utf-8';
if (this.#currentEndpointBody) {
const currentEndpointBody = this.#currentEndpointBody;
this.#currentEndpointBody = undefined;
return currentEndpointBody;
} else {
return { body: await response.text(), encoding: encoding as BufferEncoding };
}
}
}
Loading