withastro · ematipico · Mar 8, 2024 · Feb 13, 2024 · Feb 14, 2024 · Feb 14, 2024
diff --git a/.changeset/cool-jobs-fetch.md b/.changeset/cool-jobs-fetch.md
@@ -0,0 +1,11 @@
+---
+"astro": minor
+---
+
+Adds a new `experimental.directRenderScript` option.
+
+This option replaces the `experimental.optimizeHoistedScript` option to use a more reliable strategy to prevent scripts being executed in pages they are not used. The scripts are now directly rendered as declared in Astro files (with features like TypeScript and importing `node_modules` still working), and should result in scripts running in the correct pages compared to the previous static analysis approach.
+
+However, as scripts are now directly rendered, they are no longer hoisted to the `<head>` and multiple scripts on a page are no longer bundled together. If you enable this option, you should check if it affects your site's behaviour.
+
+This option will be enabled by default in Astro 5.0.
diff --git a/packages/astro/package.json b/packages/astro/package.json
@@ -114,7 +114,7 @@
     "test:node": "astro-scripts test \"test/**/*.nodetest.js\""
   },
   "dependencies": {
-    "@astrojs/compiler": "^2.5.3",
+    "@astrojs/compiler": "0.0.0-render-script-20240214150051",
     "@astrojs/internal-helpers": "workspace:*",
     "@astrojs/markdown-remark": "workspace:*",
     "@astrojs/telemetry": "workspace:*",
@@ -198,7 +198,6 @@
     "@types/diff": "^5.0.8",
     "@types/dlv": "^1.1.4",
     "@types/dom-view-transitions": "^1.0.4",
-    "@types/estree": "^1.0.5",
     "@types/hast": "^3.0.3",
     "@types/html-escaper": "^3.0.2",
     "@types/http-cache-semantics": "^4.0.4",

diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -1601,25 +1601,28 @@ export interface AstroUserConfig {
 	experimental?: {
 		/**
 		 * @docs
-		 * @name experimental.optimizeHoistedScript
+		 * @name experimental.directRenderScript
 		 * @type {boolean}
 		 * @default `false`
-		 * @version 2.10.4
+		 * @version 4.5.0
 		 * @description
-		 * Prevents unused components' scripts from being included in a page unexpectedly.
-		 * The optimization is best-effort and may inversely miss including the used scripts. Make sure to double-check your built pages
-		 * before publishing.
-		 * Enable hoisted script analysis optimization by adding the experimental flag:
+		 * Directly render scripts as declared in Astro files (with features like TypeScript and importing `node_modules` still working).
+		 * This should result in scripts running in the correct pages compared to the previous static analysis approach.
+		 *
+		 * However, as scripts are now directly rendered, they are no longer hoisted to the `<head>` and multiple scripts on a page are no
+		 * longer bundled together. If you enable this option, you should check if it affects your site's behaviour.
+		 *
+		 * This option will be enabled by default in Astro 5.0.
 		 *
 		 * ```js
 		 * {
 		 * 	experimental: {
-		 *		optimizeHoistedScript: true,
+		 *		directRenderScript: true,
 		 * 	},
 		 * }
 		 * ```
 		 */
-		optimizeHoistedScript?: boolean;
+		directRenderScript?: boolean;
 
 		/**
 		 * @docs
@@ -2691,6 +2694,7 @@ export interface SSRResult {
 	scripts: Set<SSRElement>;
 	links: Set<SSRElement>;
 	componentMetadata: Map<string, SSRComponentMetadata>;
+	inlinedScripts: Map<string, string>;
 	createAstro(
 		Astro: AstroGlobalPartial,
 		props: Record<string, any>,

diff --git a/packages/astro/src/content/vite-plugin-content-assets.ts b/packages/astro/src/content/vite-plugin-content-assets.ts
@@ -1,7 +1,7 @@
 import { extname } from 'node:path';
 import { pathToFileURL } from 'node:url';
 import type { Plugin, Rollup } from 'vite';
-import type { AstroSettings } from '../@types/astro.js';
+import type { AstroSettings, SSRElement } from '../@types/astro.js';
 import { moduleIsTopLevelPage, walkParentInfos } from '../core/build/graph.js';
 import { getPageDataByViteID, type BuildInternals } from '../core/build/internal.js';
 import type { AstroBuildPlugin } from '../core/build/plugin.js';
@@ -70,8 +70,15 @@ export function astroContentAssetPropagationPlugin({
 						crawledFiles: styleCrawledFiles,
 					} = await getStylesForURL(pathToFileURL(basePath), devModuleLoader);
 
-					const { scripts: hoistedScripts, crawledFiles: scriptCrawledFiles } =
-						await getScriptsForURL(pathToFileURL(basePath), settings.config.root, devModuleLoader);
+					// Add hoisted script tags, skip if direct rendering with `directRenderScript`
+					const { scripts: hoistedScripts, crawledFiles: scriptCrawledFiles } = settings.config
+						.experimental.directRenderScript
+						? { scripts: new Set<SSRElement>(), crawledFiles: new Set<string>() }
+						: await getScriptsForURL(
+								pathToFileURL(basePath),
+								settings.config.root,
+								devModuleLoader
+							);
 
 					// Register files we crawled to be able to retrieve the rendered styles and scripts,
 					// as when they get updated, we need to re-transform ourselves.

diff --git a/packages/astro/src/core/app/common.ts b/packages/astro/src/core/app/common.ts
@@ -15,6 +15,7 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
 
 	const assets = new Set<string>(serializedManifest.assets);
 	const componentMetadata = new Map(serializedManifest.componentMetadata);
+	const inlinedScripts = new Map(serializedManifest.inlinedScripts);
 	const clientDirectives = new Map(serializedManifest.clientDirectives);
 
 	return {
@@ -25,6 +26,7 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
 		...serializedManifest,
 		assets,
 		componentMetadata,
+		inlinedScripts,
 		clientDirectives,
 		routes,
 	};

diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -133,6 +133,7 @@ export class App {
 			compressHTML: this.#manifest.compressHTML,
 			renderers: this.#manifest.renderers,
 			clientDirectives: this.#manifest.clientDirectives,
+			inlinedScripts: this.#manifest.inlinedScripts,
 			resolve: async (specifier: string) => {
 				if (!(specifier in this.#manifest.entryModules)) {
 					throw new Error(`Unable to resolve [${specifier}]`);

diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts
@@ -50,6 +50,7 @@ export type SSRManifest = {
 	 */
 	clientDirectives: Map<string, string>;
 	entryModules: Record<string, string>;
+	inlinedScripts: Map<string, string>;
 	assets: Set<string>;
 	componentMetadata: SSRResult['componentMetadata'];
 	pageModule?: SinglePageBuiltModule;
@@ -68,10 +69,11 @@ export type SSRManifestI18n = {
 
 export type SerializedSSRManifest = Omit<
 	SSRManifest,
-	'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'clientDirectives'
+	'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'inlinedScripts' | 'clientDirectives'
 > & {
 	routes: SerializedRouteInfo[];
 	assets: string[];
 	componentMetadata: [string, SSRComponentMetadata][];
+	inlinedScripts: [string, string][];
 	clientDirectives: [string, string][];
 };
diff --git a/packages/astro/src/core/build/buildPipeline.ts b/packages/astro/src/core/build/buildPipeline.ts
@@ -39,6 +39,7 @@ export class BuildPipeline extends Pipeline {
 				mode: staticBuildOptions.mode,
 				renderers: manifest.renderers,
 				clientDirectives: manifest.clientDirectives,
+				inlinedScripts: manifest.inlinedScripts,
 				compressHTML: manifest.compressHTML,
 				async resolve(specifier: string) {
 					if (resolveCache.has(specifier)) {

diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -669,6 +669,7 @@ function createBuildManifest(
 		trailingSlash: settings.config.trailingSlash,
 		assets: new Set(),
 		entryModules: Object.fromEntries(internals.entrySpecifierToBundleMap.entries()),
+		inlinedScripts: internals.inlinedScripts,
 		routes: [],
 		adapterName: '',
 		clientDirectives: settings.clientDirectives,

diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
@@ -26,6 +26,13 @@ export interface BuildInternals {
 	// A mapping of hoisted script ids back to the pages which reference it
 	hoistedScriptIdToPagesMap: Map<string, Set<string>>;
 
+	/**
+	 * Used by the `directRenderScript` option. If script is inlined, its id and
+	 * inlined code is mapped here. The resolved id is an URL like "/_astro/something.js"
+	 * but will no longer exist as the content is now inlined in this map.
+	 */
+	inlinedScripts: Map<string, string>;
+
 	// A mapping of specifiers like astro/client/idle.js to the hashed bundled name.
 	// Used to render pages with the correct specifiers.
 	entrySpecifierToBundleMap: Map<string, string>;
@@ -115,6 +122,7 @@ export function createBuildInternals(): BuildInternals {
 		cssModuleToChunkIdMap: new Map(),
 		hoistedScriptIdToHoistedMap,
 		hoistedScriptIdToPagesMap,
+		inlinedScripts: new Map(),
 		entrySpecifierToBundleMap: new Map<string, string>(),
 		pageToBundleMap: new Map<string, string>(),
 		pagesByComponent: new Map(),

diff --git a/packages/astro/src/core/build/plugins/index.ts b/packages/astro/src/core/build/plugins/index.ts
@@ -13,6 +13,7 @@ import { pluginMiddleware } from './plugin-middleware.js';
 import { pluginPages } from './plugin-pages.js';
 import { pluginPrerender } from './plugin-prerender.js';
 import { pluginRenderers } from './plugin-renderers.js';
+import { pluginScripts } from './plugin-scripts.js';
 import { pluginSSR, pluginSSRSplit } from './plugin-ssr.js';
 
 export function registerAllPlugins({ internals, options, register }: AstroBuildPluginContainer) {
@@ -28,7 +29,11 @@ export function registerAllPlugins({ internals, options, register }: AstroBuildP
 	register(astroHeadBuildPlugin(internals));
 	register(pluginPrerender(options, internals));
 	register(astroConfigBuildPlugin(options, internals));
-	register(pluginHoistedScripts(options, internals));
+	if (options.settings.config.experimental.directRenderScript) {
+		register(pluginScripts(internals));
+	} else {
+		register(pluginHoistedScripts(options, internals));
+	}
 	register(pluginSSR(options, internals));
 	register(pluginSSRSplit(options, internals));
 	register(pluginChunks());

diff --git a/packages/astro/src/core/build/plugins/plugin-analyzer.ts b/packages/astro/src/core/build/plugins/plugin-analyzer.ts
@@ -1,10 +1,9 @@
-import type { ModuleInfo, PluginContext } from 'rollup';
+import type { PluginContext } from 'rollup';
 import type { Plugin as VitePlugin } from 'vite';
 import type { PluginMetadata as AstroPluginMetadata } from '../../../vite-plugin-astro/types.js';
 import type { BuildInternals } from '../internal.js';
 import type { AstroBuildPlugin } from '../plugin.js';
 
-import type { ExportDefaultDeclaration, ExportNamedDeclaration, ImportDeclaration } from 'estree';
 import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js';
 import { prependForwardSlash } from '../../../core/path.js';
 import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from '../graph.js';
@@ -19,110 +18,6 @@ function isPropagatedAsset(id: string) {
 	}
 }
 
-/**
- * @returns undefined if the parent does not import the child, string[] of the reexports if it does
- */
-async function doesParentImportChild(
-	this: PluginContext,
-	parentInfo: ModuleInfo,
-	childInfo: ModuleInfo | undefined,
-	childExportNames: string[] | 'dynamic' | undefined
-): Promise<'no' | 'dynamic' | string[]> {
-	if (!childInfo || !parentInfo.ast || !childExportNames) return 'no';
-
-	// If we're dynamically importing the child, return `dynamic` directly to opt-out of optimization
-	if (childExportNames === 'dynamic' || parentInfo.dynamicallyImportedIds?.includes(childInfo.id)) {
-		return 'dynamic';
-	}
-
-	const imports: Array<ImportDeclaration> = [];
-	const exports: Array<ExportNamedDeclaration | ExportDefaultDeclaration> = [];
-	for (const node of (parentInfo.ast as any).body) {
-		if (node.type === 'ImportDeclaration') {
-			imports.push(node);
-		} else if (node.type === 'ExportDefaultDeclaration' || node.type === 'ExportNamedDeclaration') {
-			exports.push(node);
-		}
-	}
-
-	// All local import names that could be importing the child component
-	const importNames: string[] = [];
-	// All of the aliases the child component is exported as
-	const exportNames: string[] = [];
-
-	// Iterate each import, find it they import the child component, if so, check if they
-	// import the child component name specifically. We can verify this with `childExportNames`.
-	for (const node of imports) {
-		const resolved = await this.resolve(node.source.value as string, parentInfo.id);
-		if (!resolved || resolved.id !== childInfo.id) continue;
-		for (const specifier of node.specifiers) {
-			// TODO: handle these?
-			if (specifier.type === 'ImportNamespaceSpecifier') continue;
-			const name =
-				specifier.type === 'ImportDefaultSpecifier' ? 'default' : specifier.imported.name;
-			// If we're importing the thing that the child exported, store the local name of what we imported
-			if (childExportNames.includes(name)) {
-				importNames.push(specifier.local.name);
-			}
-		}
-	}
-
-	// Iterate each export, find it they re-export the child component, and push the exported name to `exportNames`
-	for (const node of exports) {
-		if (node.type === 'ExportDefaultDeclaration') {
-			if (node.declaration.type === 'Identifier' && importNames.includes(node.declaration.name)) {
-				exportNames.push('default');
-			}
-		} else {
-			// Handle:
-			// export { Component } from './Component.astro'
-			// export { Component as AliasedComponent } from './Component.astro'
-			if (node.source) {
-				const resolved = await this.resolve(node.source.value as string, parentInfo.id);
-				if (!resolved || resolved.id !== childInfo.id) continue;
-				for (const specifier of node.specifiers) {
-					if (childExportNames.includes(specifier.local.name)) {
-						importNames.push(specifier.local.name);
-						exportNames.push(specifier.exported.name);
-					}
-				}
-			}
-			// Handle:
-			// export const AliasedComponent = Component
-			// export const AliasedComponent = Component, let foo = 'bar'
-			if (node.declaration) {
-				if (node.declaration.type !== 'VariableDeclaration') continue;
-				for (const declarator of node.declaration.declarations) {
-					if (declarator.init?.type !== 'Identifier') continue;
-					if (declarator.id.type !== 'Identifier') continue;
-					if (importNames.includes(declarator.init.name)) {
-						exportNames.push(declarator.id.name);
-					}
-				}
-			}
-			// Handle:
-			// export { Component }
-			// export { Component as AliasedComponent }
-			for (const specifier of node.specifiers) {
-				if (importNames.includes(specifier.local.name)) {
-					exportNames.push(specifier.exported.name);
-				}
-			}
-		}
-	}
-	if (!importNames.length) return 'no';
-
-	// If the component is imported by another component, assume it's in use
-	// and start tracking this new component now
-	if (parentInfo.id.endsWith('.astro')) {
-		exportNames.push('default');
-	} else if (parentInfo.id.endsWith('.mdx')) {
-		exportNames.push('Content');
-	}
-
-	return exportNames;
-}
-
 export function vitePluginAnalyzer(
 	options: StaticBuildOptions,
 	internals: BuildInternals
@@ -150,39 +45,9 @@ export function vitePluginAnalyzer(
 				}
 
 				if (hoistedScripts.size) {
-					// These variables are only used for hoisted script analysis optimization
-					const depthsToChildren = new Map<number, ModuleInfo>();
-					const depthsToExportNames = new Map<number, string[] | 'dynamic'>();
-					// The component export from the original component file will always be default.
-					depthsToExportNames.set(0, ['default']);
-
-					for (const [parentInfo, depth] of walkParentInfos(from, this, function until(importer) {
+					for (const [parentInfo] of walkParentInfos(from, this, function until(importer) {
 						return isPropagatedAsset(importer);
 					})) {
-						// If hoisted script analysis optimization is enabled, try to analyse and bail early if possible
-						if (options.settings.config.experimental.optimizeHoistedScript) {
-							depthsToChildren.set(depth, parentInfo);
-							// If at any point
-							if (depth > 0) {
-								// Check if the component is actually imported:
-								const childInfo = depthsToChildren.get(depth - 1);
-								const childExportNames = depthsToExportNames.get(depth - 1);
-
-								const doesImport = await doesParentImportChild.call(
-									this,
-									parentInfo,
-									childInfo,
-									childExportNames
-								);
-
-								if (doesImport === 'no') {
-									// Break the search if the parent doesn't import the child.
-									continue;
-								}
-								depthsToExportNames.set(depth, doesImport);
-							}
-						}
-
 						if (isPropagatedAsset(parentInfo.id)) {
 							for (const [nestedParentInfo] of walkParentInfos(from, this)) {
 								if (moduleIsTopLevelPage(nestedParentInfo)) {
@@ -263,7 +128,9 @@ export function vitePluginAnalyzer(
 	return {
 		name: '@astro/rollup-plugin-astro-analyzer',
 		async generateBundle() {
-			const hoistScanner = hoistedScriptScanner();
+			const hoistScanner = options.settings.config.experimental.directRenderScript
+				? { scan: async () => {}, finalize: () => {} }
+				: hoistedScriptScanner();
 
 			const ids = this.getModuleIds();
 
@@ -317,6 +184,16 @@ export function vitePluginAnalyzer(
 						trackClientOnlyPageDatas(internals, newPageData, clientOnlys);
 					}
 				}
+
+				// When directly rendering scripts, we don't need to group them together when bundling,
+				// each script module is its own entrypoint, so we directly assign each script modules to
+				// `discoveredScripts` here, which will eventually be passed as inputs of the client build.
+				if (options.settings.config.experimental.directRenderScript && astro.scripts.length) {
+					for (let i = 0; i < astro.scripts.length; i++) {
+						const hid = `${id.replace('/@fs', '')}?astro&type=script&index=${i}&lang.ts`;
+						internals.discoveredScripts.add(hid);
+					}
+				}
 			}
 
 			// Finalize hoisting