Add experimental.directRenderScript option (#10102)

* Add `experimental.directRenderScript` option * Support inlining scripts * Use compiler preview release * Address feedback * Fix * Add enable by default note * Fix build * Dedupe rendered scripts * Fix test * Update deps * Update .changeset/cool-jobs-fetch.md Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update jsdoc * Typo * resolve merge conflicts * Fix examples check fail --------- Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
withastro · Mar 8, 2024 · e3f02f5 · e3f02f5
1 parent c081adf
commit e3f02f5
Show file tree

Hide file tree

Showing 30 changed files with 239 additions and 199 deletions.
diff --git a/.changeset/cool-jobs-fetch.md b/.changeset/cool-jobs-fetch.md
@@ -0,0 +1,25 @@
+---
+"astro": minor
+---
+
+Adds a new `experimental.directRenderScript` configuration option which provides a more reliable strategy to prevent scripts from being executed in pages where they are not used. 
+
+This replaces the `experimental.optimizeHoistedScript` flag introduced in v2.10.4 to prevent unused components' scripts from being included in a page unexpectedly. That experimental option no longer exists and must be removed from your configuration, whether or not you enable `directRenderScript`:
+
+```diff
+// astro.config.mjs
+import { defineConfig } from 'astro/config';
+
+export default defineConfig({
+	experimental: {
+-		optimizeHoistedScript: true,
++		directRenderScript: true
+	}
+});
+```
+
+With `experimental.directRenderScript` configured, scripts are now directly rendered as declared in Astro files (including existing features like TypeScript, importing `node_modules`, and deduplicating scripts). You can also now conditionally render scripts in your Astro file.
+
+However, this means scripts 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 that all your `<script>` tags behave as expected.
+
+This option will be enabled by default in Astro 5.0.
diff --git a/packages/astro/package.json b/packages/astro/package.json
@@ -197,7 +197,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
@@ -1605,25 +1605,30 @@ 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:
+		 * Enables a more reliable strategy to prevent scripts from being executed in pages where they are not used.
+		 * 
+		 * Scripts will directly render as declared in Astro files (including existing features like TypeScript, importing `node_modules`,
+		 * and deduplicating scripts). You can also now conditionally render scripts in your Astro file.
+
+		 * However, this means scripts 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 that all your `<script>` tags behave as expected.
+		 *
+		 * This option will be enabled by default in Astro 5.0.
 		 *
 		 * ```js
 		 * {
-		 * 	experimental: {
-		 *		optimizeHoistedScript: true,
-		 * 	},
+		 *   experimental: {
+		 *     directRenderScript: true,
+		 *   },
 		 * }
 		 * ```
 		 */
-		optimizeHoistedScript?: boolean;
+		directRenderScript?: boolean;
 
 		/**
 		 * @docs
@@ -2743,6 +2748,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>,
@@ -2777,6 +2783,11 @@ export interface SSRMetadata {
 	 * script in the page HTML before the first Solid component.
 	 */
 	rendererSpecificHydrationScripts: Set<string>;
+	/**
+	 * Used by `renderScript` to track script ids that have been rendered,
+	 * so we only render each once.
+	 */
+	renderedScripts: Set<string>;
 	hasDirectives: Set<string>;
 	hasRenderedHead: boolean;
 	headInTree: boolean;

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 { type BuildInternals, getPageDataByViteID } 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/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/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
@@ -38,6 +38,7 @@ export abstract class Pipeline {
 		 */
 		readonly adapterName = manifest.adapterName,
 		readonly clientDirectives = manifest.clientDirectives,
+		readonly inlinedScripts = manifest.inlinedScripts,
 		readonly compressHTML = manifest.compressHTML,
 		readonly i18n = manifest.i18n,
 		readonly middleware = manifest.middleware,

diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -602,6 +602,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());