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,7 @@
+---
+"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 intact), 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>`, which you may need to check if it affects your site's behaviour.
diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -1601,25 +1601,24 @@ export interface AstroUserConfig {
 	experimental?: {
 		/**
 		 * @docs
-		 * @name experimental.optimizeHoistedScript
+		 * @name experimental.directRenderScript
 		 * @type {boolean}
 		 * @default `false`
-		 * @version 2.10.4
+		 * @version 4.4.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 processed scripts where they are declared in Astro files. The scripts will no longer be hoisted to the head.
+		 * This may change the execution order of scripts if you added other inline scripts, but will result in more accurate script
+		 * inclusion when rendering the final HTML.
 		 *
 		 * ```js
 		 * {
 		 * 	experimental: {
-		 *		optimizeHoistedScript: true,
+		 *		directRenderScript: true,
 		 * 	},
 		 * }
 		 * ```
 		 */
-		optimizeHoistedScript?: boolean;
+		directRenderScript?: boolean;
 
 		/**
 		 * @docs

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/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

diff --git a/packages/astro/src/core/compile/compile.ts b/packages/astro/src/core/compile/compile.ts
@@ -55,6 +55,7 @@ export async function compile({
 				astroConfig.devToolbar &&
 				astroConfig.devToolbar.enabled &&
 				(await preferences.get('devToolbar.enabled')),
+			renderScript: astroConfig.experimental.directRenderScript,
 			preprocessStyle: createStylePreprocessor({
 				filename,
 				viteConfig,

diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
@@ -58,7 +58,7 @@ const ASTRO_CONFIG_DEFAULTS = {
 	legacy: {},
 	redirects: {},
 	experimental: {
-		optimizeHoistedScript: false,
+		directRenderScript: false,
 		contentCollectionCache: false,
 		clientPrerender: false,
 		globalRoutePriority: false,
@@ -484,10 +484,10 @@ export const AstroConfigSchema = z.object({
 	),
 	experimental: z
 		.object({
-			optimizeHoistedScript: z
+			directRenderScript: z
 				.boolean()
 				.optional()
-				.default(ASTRO_CONFIG_DEFAULTS.experimental.optimizeHoistedScript),
+				.default(ASTRO_CONFIG_DEFAULTS.experimental.directRenderScript),
 			contentCollectionCache: z
 				.boolean()
 				.optional()

diff --git a/packages/astro/src/runtime/compiler/index.ts b/packages/astro/src/runtime/compiler/index.ts
@@ -13,6 +13,7 @@ export {
 	render,
 	renderComponent,
 	renderHead,
+	renderScript,
 	renderSlot,
 	renderTransition,
 	spreadAttributes,

diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
@@ -24,6 +24,7 @@ export {
 	renderHead,
 	renderHTMLElement,
 	renderPage,
+	renderScript,
 	renderScriptElement,
 	renderSlot,
 	renderSlotToString,

diff --git a/packages/astro/src/runtime/server/render/index.ts b/packages/astro/src/runtime/server/render/index.ts
@@ -2,6 +2,7 @@ export { createHeadAndContent, renderTemplate, renderToString } from './astro/in
 export type { AstroComponentFactory, AstroComponentInstance } from './astro/index.js';
 export { Fragment, Renderer, chunkToByteArray, chunkToString } from './common.js';
 export { renderComponent, renderComponentToString } from './component.js';
+export { renderScript } from './script.js';
 export { renderHTMLElement } from './dom.js';
 export { maybeRenderHead, renderHead } from './head.js';
 export type { RenderInstruction } from './instruction.js';

diff --git a/packages/astro/src/runtime/server/render/script.ts b/packages/astro/src/runtime/server/render/script.ts
@@ -0,0 +1,11 @@
+import type { SSRResult } from '../../../@types/astro.js';
+import { markHTMLString } from '../escape.js';
+
+/**
+ * Relies on the `renderScript: true` compiler option
+ * @experimental
+ */
+export async function renderScript(result: SSRResult, path: string) {
+	const resolved = await result.resolve(path);
+	return markHTMLString(`<script type="module" src="${resolved}"></script>`);
+}
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -394,8 +394,10 @@ async function getScriptsAndStyles({ pipeline, filePath }: GetScriptsAndStylesPa
 	const moduleLoader = pipeline.getModuleLoader();
 	const settings = pipeline.getSettings();
 	const mode = pipeline.getEnvironment().mode;
-	// Add hoisted script tags
-	const { scripts } = await getScriptsForURL(filePath, settings.config.root, moduleLoader);
+	// Add hoisted script tags, skip if direct rendering with `directRenderScript`
+	const { scripts } = settings.config.experimental.directRenderScript
+		? { scripts: new Set<SSRElement>() }
+		: await getScriptsForURL(filePath, settings.config.root, moduleLoader);
 
 	// Inject HMR scripts
 	if (isPage(filePath, settings) && mode === 'development') {

diff --git a/packages/astro/test/fixtures/hoisted-imports/astro.config.js b/packages/astro/test/fixtures/hoisted-imports/astro.config.js
@@ -0,0 +1,7 @@
+import { defineConfig } from 'astro/config';
+
+export default defineConfig({
+	experimental: {
+		directRenderScript: true,
+	},
+});
diff --git a/packages/astro/test/hoisted-imports.test.js b/packages/astro/test/hoisted-imports.test.js
@@ -8,9 +8,6 @@ describe('Hoisted Imports', () => {
 	before(async () => {
 		fixture = await loadFixture({
 			root: './fixtures/hoisted-imports/',
-			experimental: {
-				optimizeHoistedScript: true,
-			},
 		});
 	});
 
@@ -67,14 +64,7 @@ describe('Hoisted Imports', () => {
 			expectScript(scripts, 'D');
 			expectScript(scripts, 'E');
 		});
-		it('includes all imported scripts when dynamically imported', async () => {
-			const scripts = await getAllScriptText('/dynamic/index.html');
-			expectScript(scripts, 'A');
-			expectScript(scripts, 'B');
-			expectScript(scripts, 'C');
-			expectScript(scripts, 'D');
-			expectScript(scripts, 'E');
-		});
+
 		it('includes no scripts when none imported', async () => {
 			const scripts = await getAllScriptText('/none/index.html');
 			expectNotScript(scripts, 'A');
@@ -83,6 +73,7 @@ describe('Hoisted Imports', () => {
 			expectNotScript(scripts, 'D');
 			expectNotScript(scripts, 'E');
 		});
+
 		it('includes some scripts', async () => {
 			const scripts = await getAllScriptText('/some/index.html');
 			expectScript(scripts, 'A');