michael/astro
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Determine hoisted scripts via AST analysis (#7707)

Browse source 
* initial hacky

* plural importNames and exportNames

* extract into function

* Add test

* fix issue with another component importing our tracked component

* fix dynamic imports

* changeset

* add mdx to checklist

* mdx exports Content

* remove unused var

* Update packages/astro/test/hoisted-imports.test.js

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

* Update packages/astro/src/core/build/plugins/plugin-analyzer.ts

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

* Update packages/astro/src/core/build/plugins/plugin-analyzer.ts

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

* Update real-drinks-melt.md

* Update .changeset/real-drinks-melt.md

Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>

* Update .changeset/real-drinks-melt.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

* Update .changeset/real-drinks-melt.md

* Update .changeset/real-drinks-melt.md

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

---------

Co-authored-by: Matthew Phillips <matthew@skypack.dev>
Co-authored-by: Matthew Phillips <matthew@matthewphillips.info>
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
This commit is contained in:
ottomated 2023-07-20 12:03:40 -04:00 committed by GitHub
parent 7a26a52e19
commit 3a6e42e190
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 330 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
.changeset/real-drinks-melt.md Normal file
View file

@ -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.

2
packages/astro/src/core/build/graph.ts
View file

@ -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);
}
}

136
packages/astro/src/core/build/plugins/plugin-analyzer.ts
View file

@ -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[] = [];

8
packages/astro/test/fixtures/hoisted-imports/package.json vendored Normal file
View file

@ -0,0 +1,8 @@
{
"name": "@test/hoisted-imports",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

3
packages/astro/test/fixtures/hoisted-imports/src/components/A.astro vendored Normal file
View file

@ -0,0 +1,3 @@
<script>
console.log('A');
</script>

3
packages/astro/test/fixtures/hoisted-imports/src/components/B.astro vendored Normal file
View file

@ -0,0 +1,3 @@
<script>
console.log('B');
</script>

3
packages/astro/test/fixtures/hoisted-imports/src/components/C.astro vendored Normal file
View file

@ -0,0 +1,3 @@
<script>
console.log('C');
</script>

5
packages/astro/test/fixtures/hoisted-imports/src/components/CWrapper.astro vendored Normal file
View file

@ -0,0 +1,5 @@
---
import C from './C.astro';
---
<C />

3
packages/astro/test/fixtures/hoisted-imports/src/components/D.astro vendored Normal file
View file

@ -0,0 +1,3 @@
<script>
console.log('D');
</script>

3
packages/astro/test/fixtures/hoisted-imports/src/components/E.astro vendored Normal file
View file

@ -0,0 +1,3 @@
<script>
console.log('E');
</script>

9
packages/astro/test/fixtures/hoisted-imports/src/components/index.ts vendored Normal file
View file

@ -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;

12
packages/astro/test/fixtures/hoisted-imports/src/pages/all.astro vendored Normal file
View file

@ -0,0 +1,12 @@
---
import E, { A, B2, C_aliased, D_aliased } from '../components';
---
<head>
</head>
<A />
<B2 />
<C_aliased />
<D_aliased />
<E />

4
packages/astro/test/fixtures/hoisted-imports/src/pages/dynamic.astro vendored Normal file
View file

@ -0,0 +1,4 @@
---
import('../components/index');
---
<head></head>

7
packages/astro/test/fixtures/hoisted-imports/src/pages/none.astro vendored Normal file
View file

@ -0,0 +1,7 @@
---
import '../components';
---
<head>
</head>

9
packages/astro/test/fixtures/hoisted-imports/src/pages/some.astro vendored Normal file
View file

@ -0,0 +1,9 @@
---
import { A, C_aliased as C } from '../components';
---
<head>
</head>
<A />
<C />

92
packages/astro/test/hoisted-imports.test.js Normal file
View file

@ -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');
});
});
});

6
pnpm-lock.yaml
View file

@ -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: