diff options
17 files changed, 330 insertions, 5 deletions
diff --git a/.changeset/real-drinks-melt.md b/.changeset/real-drinks-melt.md new file mode 100644 index 000000000..648ef1d91 --- /dev/null +++ b/.changeset/real-drinks-melt.md @@ -0,0 +1,30 @@ +--- +'astro': minor +--- + +Improved hoisted script bundling + +Astro's static analysis to determine which `<script>` tags to bundle together just got a little smarter! + +Astro create bundles that optimize script usage between pages and place them in the head of the document so that they are downloaded as early as possible. One limitation to Astro's existing approach has been that you could not dynamically use hoisted scripts. Each page received the same, all-inclusive bundle whether or not every script was needed on that page. + +Now, Astro has improved the static analysis to take into account the actual imports used. + +For example, Astro would previously bundle the `<script>`s from both the `<Tab>` and `<Accordian>` component for the following library that re-exports multiple components: + +__@matthewp/my-astro-lib__ + +```js +export { default as Tabs } from './Tabs.astro'; +export { default as Accordion } from './Accordion.astro'; +``` + +Now, when an Astro page only uses a single component, Astro will send only the necessary script to the page. A page that only imports the `<Accordian>` component will not receive any `<Tab>` component's scripts: + +```astro +--- +import { Accordion } from '@matthewp/my-astro-lib'; +--- +``` + +You should now see more efficient performance with Astro now supporting this common library re-export pattern. diff --git a/packages/astro/src/core/build/graph.ts b/packages/astro/src/core/build/graph.ts index 3ce325309..d2c924c8d 100644 --- a/packages/astro/src/core/build/graph.ts +++ b/packages/astro/src/core/build/graph.ts @@ -35,7 +35,7 @@ export function* walkParentInfos( if (seen.has(imp)) { continue; } - yield* walkParentInfos(imp, ctx, until, ++depth, order, seen, id); + yield* walkParentInfos(imp, ctx, until, depth + 1, order, seen, id); } } diff --git a/packages/astro/src/core/build/plugins/plugin-analyzer.ts b/packages/astro/src/core/build/plugins/plugin-analyzer.ts index b650e04f1..571b533ad 100644 --- a/packages/astro/src/core/build/plugins/plugin-analyzer.ts +++ b/packages/astro/src/core/build/plugins/plugin-analyzer.ts @@ -1,4 +1,4 @@ -import type { PluginContext } from 'rollup'; +import type { ModuleInfo, PluginContext } from 'rollup'; import type { Plugin as VitePlugin } from 'vite'; import type { PluginMetadata as AstroPluginMetadata } from '../../../vite-plugin-astro/types'; import type { BuildInternals } from '../internal.js'; @@ -8,6 +8,8 @@ import { PROPAGATED_ASSET_FLAG } from '../../../content/consts.js'; import { prependForwardSlash } from '../../../core/path.js'; import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from '../graph.js'; import { getPageDataByViteID, trackClientOnlyPageDatas } from '../internal.js'; +import { walk } from 'estree-walker'; +import type { ExportDefaultDeclaration, ExportNamedDeclaration, ImportDeclaration } from 'estree'; function isPropagatedAsset(id: string) { try { @@ -17,6 +19,102 @@ 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 (childExportNames === 'dynamic' || parentInfo.dynamicallyImportedIds?.includes(childInfo.id)) { + return 'dynamic'; + } + + const imports: Array<ImportDeclaration> = []; + const exports: Array<ExportNamedDeclaration | ExportDefaultDeclaration> = []; + walk(parentInfo.ast, { + enter(node) { + if (node.type === 'ImportDeclaration') { + imports.push(node as ImportDeclaration); + } else if ( + node.type === 'ExportDefaultDeclaration' || + node.type === 'ExportNamedDeclaration' + ) { + exports.push(node as ExportNamedDeclaration | ExportDefaultDeclaration); + } + }, + }); + // All of the aliases the current component is imported as + const importNames: string[] = []; + // All of the aliases the child component is exported as + const exportNames: string[] = []; + + 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); + } + } + } + for (const node of exports) { + if (node.type === 'ExportDefaultDeclaration') { + if (node.declaration.type === 'Identifier' && importNames.includes(node.declaration.name)) { + exportNames.push('default'); + // return + } + } else { + // handle `export { x } from 'something';`, where the export and import are in the same node + 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); + } + } + } + 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); + } + } + } + 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(internals: BuildInternals): VitePlugin { function hoistedScriptScanner() { const uniqueHoistedIds = new Map<string, string>(); @@ -29,7 +127,11 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { >(); return { - scan(this: PluginContext, scripts: AstroPluginMetadata['astro']['scripts'], from: string) { + async scan( + this: PluginContext, + scripts: AstroPluginMetadata['astro']['scripts'], + from: string + ) { const hoistedScripts = new Set<string>(); for (let i = 0; i < scripts.length; i++) { const hid = `${from.replace('/@fs', '')}?astro&type=script&index=${i}&lang.ts`; @@ -37,9 +139,35 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { } if (hoistedScripts.size) { - for (const [parentInfo] of walkParentInfos(from, this, function until(importer) { + 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) { return isPropagatedAsset(importer); })) { + 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)) { @@ -146,7 +274,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { } // Scan hoisted scripts - hoistScanner.scan.call(this, astro.scripts, id); + await hoistScanner.scan.call(this, astro.scripts, id); if (astro.clientOnlyComponents.length) { const clientOnlys: string[] = []; diff --git a/packages/astro/test/fixtures/hoisted-imports/package.json b/packages/astro/test/fixtures/hoisted-imports/package.json new file mode 100644 index 000000000..b91d6150a --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/hoisted-imports", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +}
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/A.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/A.astro new file mode 100644 index 000000000..3ea5406f6 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/A.astro @@ -0,0 +1,3 @@ +<script> + console.log('A'); +</script>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/B.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/B.astro new file mode 100644 index 000000000..48e567703 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/B.astro @@ -0,0 +1,3 @@ +<script> + console.log('B'); +</script>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/C.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/C.astro new file mode 100644 index 000000000..ec8b281af --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/C.astro @@ -0,0 +1,3 @@ +<script> + console.log('C'); +</script>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/CWrapper.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/CWrapper.astro new file mode 100644 index 000000000..04345e8da --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/CWrapper.astro @@ -0,0 +1,5 @@ +--- +import C from './C.astro'; +--- + +<C />
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/D.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/D.astro new file mode 100644 index 000000000..0863f8c0d --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/D.astro @@ -0,0 +1,3 @@ +<script> + console.log('D'); +</script>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/E.astro b/packages/astro/test/fixtures/hoisted-imports/src/components/E.astro new file mode 100644 index 000000000..9c56556e5 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/E.astro @@ -0,0 +1,3 @@ +<script> + console.log('E'); +</script>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/components/index.ts b/packages/astro/test/fixtures/hoisted-imports/src/components/index.ts new file mode 100644 index 000000000..a0fea55b0 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/components/index.ts @@ -0,0 +1,9 @@ +import A_aliased from './A.astro'; +import { default as C_aliased } from './CWrapper.astro'; +import D from './D.astro'; +import E_aliased from './E.astro'; + +export { A_aliased as A, C_aliased as C_aliased }; +export { default as B2 } from './B.astro'; +export const D_aliased = D; +export default E_aliased; diff --git a/packages/astro/test/fixtures/hoisted-imports/src/pages/all.astro b/packages/astro/test/fixtures/hoisted-imports/src/pages/all.astro new file mode 100644 index 000000000..bae62dfdd --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/pages/all.astro @@ -0,0 +1,12 @@ +--- +import E, { A, B2, C_aliased, D_aliased } from '../components'; +--- + +<head> + +</head> +<A /> +<B2 /> +<C_aliased /> +<D_aliased /> +<E />
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/pages/dynamic.astro b/packages/astro/test/fixtures/hoisted-imports/src/pages/dynamic.astro new file mode 100644 index 000000000..4408a8f48 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/pages/dynamic.astro @@ -0,0 +1,4 @@ +--- +import('../components/index'); +--- +<head></head>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/pages/none.astro b/packages/astro/test/fixtures/hoisted-imports/src/pages/none.astro new file mode 100644 index 000000000..991ba42f2 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/pages/none.astro @@ -0,0 +1,7 @@ +--- +import '../components'; +--- + +<head> + +</head>
\ No newline at end of file diff --git a/packages/astro/test/fixtures/hoisted-imports/src/pages/some.astro b/packages/astro/test/fixtures/hoisted-imports/src/pages/some.astro new file mode 100644 index 000000000..e5321a8b1 --- /dev/null +++ b/packages/astro/test/fixtures/hoisted-imports/src/pages/some.astro @@ -0,0 +1,9 @@ +--- +import { A, C_aliased as C } from '../components'; +--- + +<head> + +</head> +<A /> +<C /> diff --git a/packages/astro/test/hoisted-imports.test.js b/packages/astro/test/hoisted-imports.test.js new file mode 100644 index 000000000..973d7103e --- /dev/null +++ b/packages/astro/test/hoisted-imports.test.js @@ -0,0 +1,92 @@ +import { expect } from 'chai'; +import { loadFixture } from './test-utils.js'; +import * as cheerio from 'cheerio'; + +describe('Hoisted Imports', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/hoisted-imports/', + }); + }); + + async function getAllScriptText(page) { + const html = await fixture.readFile(page); + const $ = cheerio.load(html); + const scriptText = []; + + const importRegex = /import\s*?['"]([^'"]+?)['"]/g; + async function resolveImports(text) { + const matches = text.matchAll(importRegex); + for (const match of matches) { + const importPath = match[1]; + const importText = await fixture.readFile('/_astro/' + importPath); + scriptText.push(await resolveImports(importText)); + } + return text; + } + + const scripts = $('script'); + for (let i = 0; i < scripts.length; i++) { + const src = scripts.eq(i).attr('src'); + + let text; + if (src) { + text = await fixture.readFile(src); + } else { + text = scripts.eq(i).text(); + } + scriptText.push(await resolveImports(text)); + } + return scriptText.join('\n'); + } + + describe('build', () => { + before(async () => { + await fixture.build(); + }); + + function expectScript(scripts, letter) { + const regex = new RegExp(`console.log\\(['"]${letter}['"]\\)`); + expect(scripts, 'missing component ' + letter).to.match(regex); + } + function expectNotScript(scripts, letter) { + const regex = new RegExp(`console.log\\(['"]${letter}['"]\\)`); + expect(scripts, "shouldn't include component " + letter).to.not.match(regex); + } + + it('includes all imported scripts', async () => { + const scripts = await getAllScriptText('/all/index.html'); + expectScript(scripts, 'A'); + expectScript(scripts, 'B'); + expectScript(scripts, 'C'); + 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'); + expectNotScript(scripts, 'B'); + expectNotScript(scripts, 'C'); + expectNotScript(scripts, 'D'); + expectNotScript(scripts, 'E'); + }); + it('includes some scripts', async () => { + const scripts = await getAllScriptText('/some/index.html'); + expectScript(scripts, 'A'); + expectNotScript(scripts, 'B'); + expectScript(scripts, 'C'); + expectNotScript(scripts, 'D'); + expectNotScript(scripts, 'E'); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 847dfb9a0..7736ea099 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2679,6 +2679,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/hoisted-imports: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/html-component: dependencies: astro: |