Skip to content

Commit

Permalink
Avoid generators when crawling module graph (#10529)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bluwy
bluwy authored Mar 22, 2024
1 parent 1ebd6d2 commit ca5455a
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 45 deletions.
4 changes: 2 additions & 2 deletions packages/astro/src/content/vite-plugin-content-assets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { pathToFileURL } from 'node:url';
import type { Plugin, Rollup } from 'vite';
import type { AstroSettings, SSRElement } from '../@types/astro.js';
import { getAssetsPrefix } from '../assets/utils/getAssetsPrefix.js';
import { moduleIsTopLevelPage, walkParentInfos } from '../core/build/graph.js';
import { getParentModuleInfos, moduleIsTopLevelPage } from '../core/build/graph.js';
import { type BuildInternals, getPageDataByViteID } from '../core/build/internal.js';
import type { AstroBuildPlugin } from '../core/build/plugin.js';
import type { StaticBuildOptions } from '../core/build/types.js';
Expand Down Expand Up @@ -186,7 +186,7 @@ export function astroConfigBuildPlugin(
}
} else {
for (const id of Object.keys(chunk.modules)) {
for (const [pageInfo] of walkParentInfos(id, ssrPluginContext!)) {
for (const pageInfo of getParentModuleInfos(id, ssrPluginContext!)) {
if (moduleIsTopLevelPage(pageInfo)) {
const pageViteID = pageInfo.id;
const pageData = getPageDataByViteID(internals, pageViteID);
Expand Down
16 changes: 8 additions & 8 deletions packages/astro/src/core/build/css-asset-name.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import crypto from 'node:crypto';
import npath from 'node:path';
import type { AstroSettings } from '../../@types/astro.js';
import { viteID } from '../util.js';
import { getTopLevelPages } from './graph.js';
import { getTopLevelPageModuleInfos } from './graph.js';

// These pages could be used as base names for the chunk hashed name, but they are confusing
// and should be avoided it possible
Expand All @@ -14,10 +14,10 @@ const confusingBaseNames = ['404', '500'];
// We could get rid of this and only use the createSlugger implementation, but this creates
// slightly prettier names.
export function shortHashedName(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
const parents = Array.from(getTopLevelPages(id, ctx));
const parents = getTopLevelPageModuleInfos(id, ctx);
return createNameHash(
getFirstParentId(parents),
parents.map(([page]) => page.id)
parents.map((page) => page.id)
);
}

Expand All @@ -38,9 +38,9 @@ export function createSlugger(settings: AstroSettings) {
const map = new Map<string, Map<string, number>>();
const sep = '-';
return function (id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
const parents = Array.from(getTopLevelPages(id, ctx));
const parents = Array.from(getTopLevelPageModuleInfos(id, ctx));
const allParentsKey = parents
.map(([page]) => page.id)
.map((page) => page.id)
.sort()
.join('-');
const firstParentId = getFirstParentId(parents) || indexPage;
Expand Down Expand Up @@ -90,17 +90,17 @@ export function createSlugger(settings: AstroSettings) {
* Find the first parent id from `parents` where its name is not confusing.
* Returns undefined if there's no parents.
*/
function getFirstParentId(parents: [ModuleInfo, number, number][]) {
function getFirstParentId(parents: ModuleInfo[]) {
for (const parent of parents) {
const id = parent[0].id;
const id = parent.id;
const baseName = npath.parse(id).name;
if (!confusingBaseNames.includes(baseName)) {
return id;
}
}
// If all parents are confusing, just use the first one. Or if there's no
// parents, this will return undefined.
return parents[0]?.[0].id;
return parents[0]?.id;
}

const charsToReplaceRe = /[.[\]]/g;
Expand Down
66 changes: 49 additions & 17 deletions packages/astro/src/core/build/graph.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,25 @@ import type { GetModuleInfo, ModuleInfo } from 'rollup';

import { ASTRO_PAGE_RESOLVED_MODULE_ID } from './plugins/plugin-pages.js';

interface ExtendedModuleInfo {
info: ModuleInfo;
depth: number;
order: number;
}

// This walks up the dependency graph and yields out each ModuleInfo object.
export function* walkParentInfos(
export function getParentExtendedModuleInfos(
id: string,
ctx: { getModuleInfo: GetModuleInfo },
until?: (importer: string) => boolean,
depth = 0,
order = 0,
childId = '',
seen = new Set<string>(),
childId = ''
): Generator<[ModuleInfo, number, number], void, unknown> {
accumulated: ExtendedModuleInfo[] = []
): ExtendedModuleInfo[] {
seen.add(id);

const info = ctx.getModuleInfo(id);
if (info) {
if (childId) {
Expand All @@ -26,17 +34,45 @@ export function* walkParentInfos(
order += idx;
}
}
accumulated.push({ info, depth, order });
}

yield [info, depth, order];
if (info && !until?.(id)) {
const importers = info.importers.concat(info.dynamicImporters);
for (const imp of importers) {
if (!seen.has(imp)) {
getParentExtendedModuleInfos(imp, ctx, until, depth + 1, order, id, seen, accumulated);
}
}
}
if (until?.(id)) return;
const importers = (info?.importers || []).concat(info?.dynamicImporters || []);
for (const imp of importers) {
if (seen.has(imp)) {
continue;

return accumulated;
}

export function getParentModuleInfos(
id: string,
ctx: { getModuleInfo: GetModuleInfo },
until?: (importer: string) => boolean,
seen = new Set<string>(),
accumulated: ModuleInfo[] = []
): ModuleInfo[] {
seen.add(id);

const info = ctx.getModuleInfo(id);
if (info) {
accumulated.push(info);
}

if (info && !until?.(id)) {
const importers = info.importers.concat(info.dynamicImporters);
for (const imp of importers) {
if (!seen.has(imp)) {
getParentModuleInfos(imp, ctx, until, seen, accumulated);
}
}
yield* walkParentInfos(imp, ctx, until, depth + 1, order, seen, id);
}

return accumulated;
}

// Returns true if a module is a top-level page. We determine this based on whether
Expand All @@ -50,13 +86,9 @@ export function moduleIsTopLevelPage(info: ModuleInfo): boolean {

// This function walks the dependency graph, going up until it finds a page component.
// This could be a .astro page, a .markdown or a .md (or really any file extension for markdown files) page.
export function* getTopLevelPages(
export function getTopLevelPageModuleInfos(
id: string,
ctx: { getModuleInfo: GetModuleInfo }
): Generator<[ModuleInfo, number, number], void, unknown> {
for (const res of walkParentInfos(id, ctx)) {
if (moduleIsTopLevelPage(res[0])) {
yield res;
}
}
): ModuleInfo[] {
return getParentModuleInfos(id, ctx).filter(moduleIsTopLevelPage);
}
14 changes: 8 additions & 6 deletions packages/astro/src/core/build/plugins/plugin-analyzer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import type { AstroBuildPlugin } from '../plugin.js';

import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
import { prependForwardSlash } from '../../../core/path.js';
import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from '../graph.js';
import {
getParentModuleInfos,
getTopLevelPageModuleInfos,
moduleIsTopLevelPage,
} from '../graph.js';
import { getPageDataByViteID, trackClientOnlyPageDatas } from '../internal.js';
import type { StaticBuildOptions } from '../types.js';

Expand Down Expand Up @@ -45,11 +49,9 @@ export function vitePluginAnalyzer(
}

if (hoistedScripts.size) {
for (const [parentInfo] of walkParentInfos(from, this, function until(importer) {
return isPropagatedAsset(importer);
})) {
for (const parentInfo of getParentModuleInfos(from, this, isPropagatedAsset)) {
if (isPropagatedAsset(parentInfo.id)) {
for (const [nestedParentInfo] of walkParentInfos(from, this)) {
for (const nestedParentInfo of getParentModuleInfos(from, this)) {
if (moduleIsTopLevelPage(nestedParentInfo)) {
for (const hid of hoistedScripts) {
if (!pageScripts.has(nestedParentInfo.id)) {
Expand Down Expand Up @@ -177,7 +179,7 @@ export function vitePluginAnalyzer(
}
}

for (const [pageInfo] of getTopLevelPages(id, this)) {
for (const pageInfo of getTopLevelPageModuleInfos(id, this)) {
const newPageData = getPageDataByViteID(internals, pageInfo.id);
if (!newPageData) continue;

Expand Down
18 changes: 10 additions & 8 deletions packages/astro/src/core/build/plugins/plugin-css.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import type { PageBuildData, StaticBuildOptions, StylesheetAsset } from '../type
import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
import type { AstroPluginCssMetadata } from '../../../vite-plugin-astro/index.js';
import * as assetName from '../css-asset-name.js';
import { moduleIsTopLevelPage, walkParentInfos } from '../graph.js';
import {
getParentExtendedModuleInfos,
getParentModuleInfos,
moduleIsTopLevelPage,
} from '../graph.js';
import {
eachPageData,
getPageDataByViteID,
Expand Down Expand Up @@ -90,9 +94,8 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
return internals.cssModuleToChunkIdMap.get(id)!;
}

for (const [pageInfo] of walkParentInfos(id, {
getModuleInfo: meta.getModuleInfo,
})) {
const ctx = { getModuleInfo: meta.getModuleInfo };
for (const pageInfo of getParentModuleInfos(id, ctx)) {
if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) {
// Split delayed assets to separate modules
// so they can be injected where needed
Expand Down Expand Up @@ -141,16 +144,15 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {

// For this CSS chunk, walk parents until you find a page. Add the CSS to that page.
for (const id of Object.keys(chunk.modules)) {
for (const [pageInfo, depth, order] of walkParentInfos(
for (const { info: pageInfo, depth, order } of getParentExtendedModuleInfos(
id,
this,
function until(importer) {
return new URL(importer, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG);
}
)) {
if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) {
for (const parent of walkParentInfos(id, this)) {
const parentInfo = parent[0];
for (const parentInfo of getParentModuleInfos(id, this)) {
if (moduleIsTopLevelPage(parentInfo) === false) continue;

const pageViteID = parentInfo.id;
Expand Down Expand Up @@ -320,7 +322,7 @@ function* getParentClientOnlys(
ctx: { getModuleInfo: GetModuleInfo },
internals: BuildInternals
): Generator<PageBuildData, void, unknown> {
for (const [info] of walkParentInfos(id, ctx)) {
for (const info of getParentModuleInfos(id, ctx)) {
yield* getPageDatasByClientOnlyID(internals, info.id);
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/src/vite-plugin-head/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { SSRComponentMetadata, SSRResult } from '../@types/astro.js';
import type { AstroBuildPlugin } from '../core/build/plugin.js';
import type { PluginMetadata } from '../vite-plugin-astro/types.js';

import { getTopLevelPages, walkParentInfos } from '../core/build/graph.js';
import { getParentModuleInfos, getTopLevelPageModuleInfos } from '../core/build/graph.js';
import type { BuildInternals } from '../core/build/internal.js';
import { getAstroMetadata } from '../vite-plugin-astro/index.js';

Expand Down Expand Up @@ -130,13 +130,13 @@ export function astroHeadBuildPlugin(internals: BuildInternals): AstroBuildPlugi
if (modinfo) {
const meta = getAstroMetadata(modinfo);
if (meta?.containsHead) {
for (const [pageInfo] of getTopLevelPages(id, this)) {
for (const pageInfo of getTopLevelPageModuleInfos(id, this)) {
let metadata = getOrCreateMetadata(pageInfo.id);
metadata.containsHead = true;
}
}
if (meta?.propagation === 'self') {
for (const [info] of walkParentInfos(id, this)) {
for (const info of getParentModuleInfos(id, this)) {
let metadata = getOrCreateMetadata(info.id);
if (metadata.propagation !== 'self') {
metadata.propagation = 'in-tree';
Expand All @@ -147,7 +147,7 @@ export function astroHeadBuildPlugin(internals: BuildInternals): AstroBuildPlugi

// Head propagation (aka bubbling)
if (mod.code && injectExp.test(mod.code)) {
for (const [info] of walkParentInfos(id, this)) {
for (const info of getParentModuleInfos(id, this)) {
getOrCreateMetadata(info.id).propagation = 'in-tree';
}
}
Expand Down

0 comments on commit ca5455a

Please sign in to comment.