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

Support re-exporting astro components containing client components #3625

Merged
merged 20 commits into from
Jun 21, 2022
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
11 changes: 11 additions & 0 deletions .changeset/cyan-kids-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'astro': patch
---

Significantly improved build performance

This change reflects in a significantly improved build performance, especially on larger sites.

With this change Astro is not building everything by statically analyzing `.astro` files. This means it no longer needs to dynamically *run* your code in order to know what JavaScript needs to be built.

With one particular large site we found it to build __32%__ faster.
9 changes: 9 additions & 0 deletions .changeset/unlucky-hairs-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@astrojs/lit': minor
---

Conform to Constructor based rendering

This changes `@astrojs/lit` to conform to the way rendering happens in all other frameworks. Instead of using the tag name `<my-element client:load>` you use the imported constructor function, `<MyElement client:load>` like you would do with any other framework.

Support for `tag-name` syntax had to be removed due to the fact that it was a runtime feature that was not statically analyzable. To improve build performance, we have removed all runtime based component discovery. Using the imported Constructor name allows Astro to discover what components need to be built and bundled for production without ever running your file.
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"test:e2e:match": "playwright test -g"
},
"dependencies": {
"@astrojs/compiler": "^0.15.2",
"@astrojs/compiler": "^0.16.1",
"@astrojs/language-server": "^0.13.4",
"@astrojs/markdown-remark": "^0.11.2",
"@astrojs/prism": "0.4.1",
Expand Down
14 changes: 14 additions & 0 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ function* throttle(max: number, inPaths: string[]) {
}
}

function shouldSkipDraft(pageModule: ComponentInstance, astroConfig: AstroConfig): boolean {
return (
// Drafts are disabled
!astroConfig.markdown.drafts &&
// This is a draft post
('frontmatter' in pageModule && (pageModule as any).frontmatter.draft === true)
);
}

// Gives back a facadeId that is relative to the root.
// ie, src/pages/index.astro instead of /Users/name..../src/pages/index.astro
export function rootRelativeFacadeId(facadeId: string, astroConfig: AstroConfig): string {
Expand Down Expand Up @@ -124,6 +133,11 @@ async function generatePage(
);
}

if(shouldSkipDraft(pageModule, opts.astroConfig)) {
info(opts.logging, null, `${magenta('⚠️')} Skipping draft ${pageData.route.component}`);
return;
}

const generationOptions: Readonly<GeneratePathOptions> = {
pageData,
internals,
Expand Down
36 changes: 36 additions & 0 deletions packages/astro/src/core/build/graph.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { GetModuleInfo, ModuleInfo, OutputChunk } from 'rollup';
import { resolvedPagesVirtualModuleId } from '../app/index.js';

// This walks up the dependency graph and yields out each ModuleInfo object.
export function* walkParentInfos(
id: string,
ctx: { getModuleInfo: GetModuleInfo },
seen = new Set<string>()
): Generator<ModuleInfo, void, unknown> {
seen.add(id);
const info = ctx.getModuleInfo(id);
if (info) {
yield info;
}
const importers = (info?.importers || []).concat(info?.dynamicImporters || []);
for (const imp of importers) {
if (seen.has(imp)) {
continue;
}
yield* walkParentInfos(imp, ctx, seen);
}
}

// This function walks the dependency graph, going up until it finds a page component.
// This could be a .astro page or a .md page.
export function* getTopLevelPages(
id: string,
ctx: { getModuleInfo: GetModuleInfo }
): Generator<string, void, unknown> {
for (const info of walkParentInfos(id, ctx)) {
const importers = (info?.importers || []).concat(info?.dynamicImporters || []);
if (importers.length <= 2 && importers[0] === resolvedPagesVirtualModuleId) {
yield info.id;
}
}
}
12 changes: 0 additions & 12 deletions packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,6 @@ class AstroBuilder {
ssr: isBuildingToSSR(this.config),
});

// Filter pages by using conditions based on their frontmatter.
Object.entries(allPages).forEach(([page, data]) => {
if ('frontmatter' in data.preload[1]) {
// TODO: add better type inference to data.preload[1]
const frontmatter = (data.preload[1] as any).frontmatter;
if (Boolean(frontmatter.draft) && !this.config.markdown.drafts) {
debug('build', timerMessage(`Skipping draft page ${page}`, this.timer.loadStart));
delete allPages[page];
}
}
});
Comment on lines -117 to -127
Copy link
Member

Choose a reason for hiding this comment

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

Does this break drafts support? Didn't see where this was handled elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I was thinking this was handled in the markdown plugin but I don't see the code for it. There are tests which are passing though. Will investigate more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test was just wrong, so this is broken in this branch. Will fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2dc17d0


debug('build', timerMessage('All pages loaded', this.timer.loadStart));

// The names of each pages
Expand Down
28 changes: 27 additions & 1 deletion packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { RenderedChunk } from 'rollup';
import type { OutputChunk, RenderedChunk } from 'rollup';
import type { PageBuildData, ViteID } from './types';

import { prependForwardSlash } from '../path.js';
Expand Down Expand Up @@ -31,6 +31,27 @@ export interface BuildInternals {
* A map for page-specific information by a client:only component
*/
pagesByClientOnly: Map<string, Set<PageBuildData>>;

/**
* A list of hydrated components that are discovered during the SSR build
* These will be used as the top-level entrypoints for the client build.
*/
discoveredHydratedComponents: Set<string>;
/**
* A list of client:only components that are discovered during the SSR build
* These will be used as the top-level entrypoints for the client build.
*/
discoveredClientOnlyComponents: Set<string>;
/**
* A list of hoisted scripts that are discovered during the SSR build
* These will be used as the top-level entrypoints for the client build.
*/
discoveredScripts: Set<string>;

// A list of all static files created during the build. Used for SSR.
staticFiles: Set<string>;
// The SSR entry chunk. Kept in internals to share between ssr/client build steps
ssrEntryChunk?: OutputChunk;
}

/**
Expand Down Expand Up @@ -64,6 +85,11 @@ export function createBuildInternals(): BuildInternals {
pagesByComponent: new Map(),
pagesByViteID: new Map(),
pagesByClientOnly: new Map(),

discoveredHydratedComponents: new Set(),
discoveredClientOnlyComponents: new Set(),
discoveredScripts: new Set(),
staticFiles: new Set(),
};
}

Expand Down
41 changes: 12 additions & 29 deletions packages/astro/src/core/build/page-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,30 +71,18 @@ export async function collectPagesData(
css: new Set(),
hoistedScript: undefined,
scripts: new Set(),
preload: await ssrPreload({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This removes the "preload" step which was the expensive part of the build that we no longer need to do. Metadata is extracted via the compiler instead.

astroConfig,
filePath: new URL(`./${route.component}`, astroConfig.root),
viteServer,
})
.then((routes) => {
clearInterval(routeCollectionLogTimeout);
if (buildMode === 'static') {
const html = `${route.pathname}`.replace(/\/?$/, '/index.html');
debug(
'build',
`├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.yellow(html)}`
);
} else {
debug('build', `├── ${colors.bold(colors.green('✔'))} ${route.component}`);
}
return routes;
})
.catch((err) => {
clearInterval(routeCollectionLogTimeout);
debug('build', `├── ${colors.bold(colors.red('✘'))} ${route.component}`);
throw err;
}),
};

clearInterval(routeCollectionLogTimeout);
if (buildMode === 'static') {
const html = `${route.pathname}`.replace(/\/?$/, '/index.html');
debug(
'build',
`├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.yellow(html)}`
);
} else {
debug('build', `├── ${colors.bold(colors.green('✔'))} ${route.component}`);
}
continue;
}
// dynamic route:
Expand Down Expand Up @@ -144,12 +132,7 @@ export async function collectPagesData(
moduleSpecifier: '',
css: new Set(),
hoistedScript: undefined,
scripts: new Set(),
preload: await ssrPreload({
astroConfig,
filePath: new URL(`./${route.component}`, astroConfig.root),
viteServer,
}),
scripts: new Set()
};
}

Expand Down
87 changes: 17 additions & 70 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import * as vite from 'vite';
import {
BuildInternals,
createBuildInternals,
trackClientOnlyPageDatas,
} from '../../core/build/internal.js';
import { prependForwardSlash } from '../../core/path.js';
import { emptyDir, removeDir } from '../../core/util.js';
Expand All @@ -23,24 +22,21 @@ import { getTimeStat } from './util.js';
import { vitePluginHoistedScripts } from './vite-plugin-hoisted-scripts.js';
import { vitePluginInternals } from './vite-plugin-internals.js';
import { vitePluginPages } from './vite-plugin-pages.js';
import { vitePluginSSR } from './vite-plugin-ssr.js';
import { vitePluginSSR, injectManifest } from './vite-plugin-ssr.js';
import { vitePluginAnalyzer } from './vite-plugin-analyzer.js';

export async function staticBuild(opts: StaticBuildOptions) {
const { allPages, astroConfig } = opts;

// The pages to be built for rendering purposes.
const pageInput = new Set<string>();

// The JavaScript entrypoints.
const jsInput = new Set<string>();

// A map of each page .astro file, to the PageBuildData which contains information
// about that page, such as its paths.
const facadeIdToPageDataMap = new Map<string, PageBuildData>();

// Build internals needed by the CSS plugin
const internals = createBuildInternals();
const uniqueHoistedIds = new Map<string, string>();

const timer: Record<string, number> = {};

Expand All @@ -53,66 +49,6 @@ export async function staticBuild(opts: StaticBuildOptions) {
// Track the page data in internals
trackPageData(internals, component, pageData, astroModuleId, astroModuleURL);

if (pageData.route.type === 'page') {
const [renderers, mod] = pageData.preload;
const metadata = mod.$$metadata;

const topLevelImports = new Set([
// The client path for each renderer
...renderers
.filter((renderer) => !!renderer.clientEntrypoint)
.map((renderer) => renderer.clientEntrypoint!),
]);

if (metadata) {
// Any component that gets hydrated
// 'components/Counter.jsx'
// { 'components/Counter.jsx': 'counter.hash.js' }
for (const hydratedComponentPath of metadata.hydratedComponentPaths()) {
topLevelImports.add(hydratedComponentPath);
}

// Track client:only usage so we can map their CSS back to the Page they are used in.
const clientOnlys = Array.from(metadata.clientOnlyComponentPaths());
trackClientOnlyPageDatas(internals, pageData, clientOnlys);

// Client-only components
for (const clientOnly of clientOnlys) {
topLevelImports.add(clientOnly);
}

// Add hoisted scripts
const hoistedScripts = new Set(metadata.hoistedScriptPaths());
if (hoistedScripts.size) {
const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort());
let moduleId: string;

// If we're already tracking this set of hoisted scripts, get the unique id
if (uniqueHoistedIds.has(uniqueHoistedId)) {
moduleId = uniqueHoistedIds.get(uniqueHoistedId)!;
} else {
// Otherwise, create a unique id for this set of hoisted scripts
moduleId = `/astro/hoisted.js?q=${uniqueHoistedIds.size}`;
uniqueHoistedIds.set(uniqueHoistedId, moduleId);
}
topLevelImports.add(moduleId);

// Make sure to track that this page uses this set of hoisted scripts
if (internals.hoistedScriptIdToPagesMap.has(moduleId)) {
const pages = internals.hoistedScriptIdToPagesMap.get(moduleId);
pages!.add(astroModuleId);
} else {
internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId]));
internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts);
}
}
}

for (const specifier of topLevelImports) {
jsInput.add(specifier);
}
}

pageInput.add(astroModuleId);
facadeIdToPageDataMap.set(fileURLToPath(astroModuleURL), pageData);
}
Expand All @@ -122,10 +58,6 @@ export async function staticBuild(opts: StaticBuildOptions) {
// condition, so we are doing it ourselves
emptyDir(astroConfig.outDir, new Set('.git'));

timer.clientBuild = performance.now();
// Run client build first, so the assets can be fed into the SSR rendered version.
await clientBuild(opts, internals, jsInput);

// Build your project (SSR application code, assets, client JS, etc.)
timer.ssr = performance.now();
info(
Expand All @@ -138,6 +70,17 @@ export async function staticBuild(opts: StaticBuildOptions) {
const ssrResult = (await ssrBuild(opts, internals, pageInput)) as RollupOutput;
info(opts.logging, 'build', dim(`Completed in ${getTimeStat(timer.ssr, performance.now())}.`));

const clientInput = new Set<string>([
...internals.discoveredHydratedComponents,
...internals.discoveredClientOnlyComponents,
...astroConfig._ctx.renderers.map(r => r.clientEntrypoint).filter(a => a) as string[],
...internals.discoveredScripts,
]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all of the client pieces to build; hydrated and client:only components, renderers, and hoisted scripts.


// Run client build first, so the assets can be fed into the SSR rendered version.
timer.clientBuild = performance.now();
await clientBuild(opts, internals, clientInput);

timer.generate = performance.now();
if (opts.buildConfig.staticMode) {
try {
Expand All @@ -146,6 +89,9 @@ export async function staticBuild(opts: StaticBuildOptions) {
await cleanSsrOutput(opts);
}
} else {
// Inject the manifest
await injectManifest(opts, internals)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SSR manifest is injected after both builds, so that we can build a full manifest containing all of the asset information.


info(opts.logging, null, `\n${bgMagenta(black(' finalizing server assets '))}\n`);
await ssrMoveAssets(opts);
}
Expand Down Expand Up @@ -198,6 +144,7 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
// SSR needs to be last
isBuildingToSSR(opts.astroConfig) &&
vitePluginSSR(opts, internals, opts.astroConfig._ctx.adapter!),
vitePluginAnalyzer(opts.astroConfig, internals)
],
publicDir: ssr ? false : viteConfig.publicDir,
root: viteConfig.root,
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/build/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {
} from '../../@types/astro';
import type { ViteConfigWithSSR } from '../create-vite';
import type { LogOptions } from '../logger/core';
import type { ComponentPreload } from '../render/dev/index';
import type { RouteCache } from '../render/route-cache';

export type ComponentPath = string;
Expand All @@ -17,7 +16,6 @@ export type ViteID = string;
export interface PageBuildData {
component: ComponentPath;
paths: string[];
preload: ComponentPreload;
route: RouteData;
moduleSpecifier: string;
css: Set<string>;
Expand Down