Inline small hoisted scripts (#3658)

* Inline small hoisted scripts

This makes it so that small hoisted scripts get inlined into the page rather than be fetched externally.

* Ensure we don't inline when there are imports

* Fix ts

* Update tests with new url structure

* Adds a changeset
This commit is contained in:
Matthew Phillips 2022-06-22 12:02:42 -04:00 committed by GitHub
parent ae7415612e
commit aeab890971
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 129 additions and 37 deletions

View file

@ -0,0 +1,7 @@
---
'astro': patch
---
Inlines small hoisted scripts
This enables a perf improvement, whereby small hoisted scripts without dependencies are inlined into the HTML, rather than loaded externally. This uses `vite.build.assetInlineLimit` to determine if the script should be inlined.

View file

@ -3,6 +3,7 @@ import type {
EndpointHandler,
ManifestData,
RouteData,
SSRElement,
} from '../../@types/astro';
import type { LogOptions } from '../logger/core.js';
import type { RouteInfo, SSRManifest as Manifest } from './types';
@ -16,6 +17,7 @@ import { RouteCache } from '../render/route-cache.js';
import {
createLinkStylesheetElementSet,
createModuleScriptElementWithSrcSet,
createModuleScriptElement,
} from '../render/ssr-element.js';
import { matchRoute } from '../routing/match.js';
export { deserializeManifest } from './common.js';
@ -79,18 +81,17 @@ export class App {
const info = this.#routeDataToRouteInfo.get(routeData!)!;
const links = createLinkStylesheetElementSet(info.links, manifest.site);
const filteredScripts = info.scripts.filter(
(script) => typeof script === 'string' || script?.stage !== 'head-inline'
) as string[];
const scripts = createModuleScriptElementWithSrcSet(filteredScripts, manifest.site);
// Add all injected scripts to the page.
let scripts = new Set<SSRElement>();
for (const script of info.scripts) {
if (typeof script !== 'string' && script.stage === 'head-inline') {
scripts.add({
props: {},
children: script.children,
});
if (('stage' in script)) {
if(script.stage === 'head-inline') {
scripts.add({
props: {},
children: script.children,
});
}
} else {
scripts.add(createModuleScriptElement(script, manifest.site));
}
}

View file

@ -12,7 +12,13 @@ export interface RouteInfo {
routeData: RouteData;
file: string;
links: string[];
scripts: Array<string | { children: string; stage: string }>;
scripts:
(
// Integration injected
{ children: string; stage: string } |
// Hoisted
{ type: 'inline' | 'external'; value: string; }
)[];
}
export type SerializedRouteInfo = Omit<RouteInfo, 'routeData'> & {

View file

@ -18,7 +18,7 @@ import { debug, info } from '../logger/core.js';
import { render } from '../render/core.js';
import {
createLinkStylesheetElementSet,
createModuleScriptElementWithSrcSet,
createModuleScriptsSet,
} from '../render/ssr-element.js';
import { createRequest } from '../request.js';
import { getOutputFilename, isBuildingToSSR } from '../util.js';
@ -124,7 +124,7 @@ async function generatePage(
const pageInfo = getPageDataByComponent(internals, pageData.route.component);
const linkIds: string[] = Array.from(pageInfo?.css ?? []);
const hoistedId = pageInfo?.hoistedScript ?? null;
const scripts = pageInfo?.hoistedScript ?? null;
const pageModule = ssrEntry.pageMap.get(pageData.component);
@ -143,7 +143,7 @@ async function generatePage(
pageData,
internals,
linkIds,
hoistedId,
scripts,
mod: pageModule,
renderers,
};
@ -167,7 +167,7 @@ interface GeneratePathOptions {
pageData: PageBuildData;
internals: BuildInternals;
linkIds: string[];
hoistedId: string | null;
scripts: { type: 'inline' | 'external', value: string } | null;
mod: ComponentInstance;
renderers: SSRLoadedRenderer[];
}
@ -182,7 +182,7 @@ async function generatePath(
gopts: GeneratePathOptions
) {
const { astroConfig, logging, origin, routeCache } = opts;
const { mod, internals, linkIds, hoistedId, pageData, renderers } = gopts;
const { mod, internals, linkIds, scripts: hoistedScripts, pageData, renderers } = gopts;
// This adds the page name to the array so it can be shown as part of stats.
if (pageData.route.type === 'page') {
@ -198,7 +198,7 @@ async function generatePath(
? joinPaths(astroConfig.site?.toString() || 'http://localhost/', astroConfig.base)
: astroConfig.site;
const links = createLinkStylesheetElementSet(linkIds.reverse(), site);
const scripts = createModuleScriptElementWithSrcSet(hoistedId ? [hoistedId] : [], site);
const scripts = createModuleScriptsSet(hoistedScripts ? [hoistedScripts] : [], site);
// Add all injected scripts to the page.
for (const script of astroConfig._ctx.scripts) {

View file

@ -69,7 +69,6 @@ export async function collectPagesData(
moduleSpecifier: '',
css: new Set(),
hoistedScript: undefined,
scripts: new Set(),
};
clearInterval(routeCollectionLogTimeout);
@ -131,7 +130,6 @@ export async function collectPagesData(
moduleSpecifier: '',
css: new Set(),
hoistedScript: undefined,
scripts: new Set(),
};
}

View file

@ -117,8 +117,8 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
output: {
format: 'esm',
entryFileNames: opts.buildConfig.serverEntry,
chunkFileNames: 'chunks/chunk.[hash].mjs',
assetFileNames: 'assets/asset.[hash][extname]',
chunkFileNames: 'chunks/[name].[hash].mjs',
assetFileNames: 'assets/[name].[hash][extname]',
...viteConfig.build?.rollupOptions?.output,
},
},
@ -202,9 +202,9 @@ async function clientBuild(
input: Array.from(input),
output: {
format: 'esm',
entryFileNames: 'entry.[hash].js',
chunkFileNames: 'chunks/chunk.[hash].js',
assetFileNames: 'assets/asset.[hash][extname]',
entryFileNames: '[name].[hash].js',
chunkFileNames: 'chunks/[name].[hash].js',
assetFileNames: 'assets/[name].[hash][extname]',
...viteConfig.build?.rollupOptions?.output,
},
preserveEntrySignatures: 'exports-only',

View file

@ -19,8 +19,7 @@ export interface PageBuildData {
route: RouteData;
moduleSpecifier: string;
css: Set<string>;
hoistedScript: string | undefined;
scripts: Set<string>;
hoistedScript: { type: 'inline' | 'external', value: string } | undefined;
}
export type AllPagesData = Record<ComponentPath, PageBuildData>;

View file

@ -40,6 +40,8 @@ export function vitePluginHoistedScripts(
},
async generateBundle(_options, bundle) {
let assetInlineLimit = astroConfig.vite?.build?.assetsInlineLimit || 4096;
// Find all page entry points and create a map of the entry point to the hashed hoisted script.
// This is used when we render so that we can add the script to the head.
for (const [id, output] of Object.entries(bundle)) {
@ -48,15 +50,34 @@ export function vitePluginHoistedScripts(
output.facadeModuleId &&
virtualHoistedEntry(output.facadeModuleId)
) {
const canBeInlined = output.imports.length === 0 && output.dynamicImports.length === 0 &&
Buffer.byteLength(output.code) <= assetInlineLimit;
let removeFromBundle = false;
const facadeId = output.facadeModuleId!;
const pages = internals.hoistedScriptIdToPagesMap.get(facadeId)!;
for (const pathname of pages) {
const vid = viteID(new URL('.' + pathname, astroConfig.root));
const pageInfo = getPageDataByViteID(internals, vid);
if (pageInfo) {
pageInfo.hoistedScript = id;
if(canBeInlined) {
pageInfo.hoistedScript = {
type: 'inline',
value: output.code
};
removeFromBundle = true;
} else {
pageInfo.hoistedScript = {
type: 'external',
value: id
};
}
}
}
// Remove the bundle if it was inlined
if(removeFromBundle) {
delete bundle[id];
}
}
}
},

View file

@ -129,7 +129,7 @@ function buildManifest(
const routes: SerializedRouteInfo[] = [];
for (const pageData of eachPageData(internals)) {
const scripts = Array.from(pageData.scripts);
const scripts: SerializedRouteInfo['scripts'] = [];
if (pageData.hoistedScript) {
scripts.unshift(pageData.hoistedScript);
}

View file

@ -25,6 +25,19 @@ export function createLinkStylesheetElementSet(hrefs: string[], site?: string) {
return new Set<SSRElement>(hrefs.map((href) => createLinkStylesheetElement(href, site)));
}
export function createModuleScriptElement(script: { type: 'inline' | 'external'; value: string; }, site?: string): SSRElement {
if(script.type === 'external') {
return createModuleScriptElementWithSrc(script.value, site);
} else {
return {
props: {
type: 'module',
},
children: script.value,
};
}
}
export function createModuleScriptElementWithSrc(src: string, site?: string): SSRElement {
return {
props: {
@ -41,3 +54,10 @@ export function createModuleScriptElementWithSrcSet(
): Set<SSRElement> {
return new Set<SSRElement>(srces.map((src) => createModuleScriptElementWithSrc(src, site)));
}
export function createModuleScriptsSet(
scripts: { type: 'inline' | 'external'; value: string; }[],
site?: string
): Set<SSRElement> {
return new Set<SSRElement>(scripts.map(script => createModuleScriptElement(script, site)));
}

View file

@ -36,7 +36,7 @@ describe('Dynamic components', () => {
expect($('astro-island').html()).to.equal('');
// test 2: component url
const href = $('astro-island').attr('component-url');
expect(href).to.include(`/entry`);
expect(href).to.include(`/PersistentCounter`);
});
});
@ -75,6 +75,6 @@ describe('Dynamic components subpath', () => {
expect($('astro-island').html()).to.equal('');
// test 2: has component url
const attr = $('astro-island').attr('component-url');
expect(attr).to.include(`blog/entry`);
expect(attr).to.include(`blog/PersistentCounter`);
});
});

View file

@ -37,15 +37,16 @@ describe('Scripts (hoisted and not)', () => {
// Inline page
let inline = await fixture.readFile('/inline/index.html');
let $ = cheerio.load(inline);
let $el = $('script');
// test 1: Just one entry module
expect($('script')).to.have.lengthOf(1);
expect($el).to.have.lengthOf(1);
// test 2: attr removed
expect($('script').attr('data-astro')).to.equal(undefined);
expect($el.attr('data-astro')).to.equal(undefined);
const entryURL = $('script').attr('src');
const inlineEntryJS = await fixture.readFile(entryURL);
expect($el.attr('src')).to.equal(undefined);
const inlineEntryJS = $el.text();
// test 3: the JS exists
expect(inlineEntryJS).to.be.ok;
@ -57,6 +58,14 @@ describe('Scripts (hoisted and not)', () => {
);
});
it('Inline scripts that are shared by multiple pages create chunks, and aren\'t inlined into the HTML', async () => {
let html = await fixture.readFile('/inline-shared-one/index.html');
let $ = cheerio.load(html);
expect($('script')).to.have.lengthOf(1);
expect($('script').attr('src')).to.not.equal(undefined);
});
it('External page builds the hoisted scripts to a single bundle', async () => {
let external = await fixture.readFile('/external/index.html');
let $ = cheerio.load(external);
@ -65,8 +74,8 @@ describe('Scripts (hoisted and not)', () => {
expect($('script')).to.have.lengthOf(2);
let el = $('script').get(1);
let entryURL = $(el).attr('src');
let externalEntryJS = await fixture.readFile(entryURL);
expect($(el).attr('src')).to.equal(undefined, 'This should have been inlined');
let externalEntryJS = $(el).text();
// test 2: the JS exists
expect(externalEntryJS).to.be.ok;

View file

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

View file

@ -0,0 +1,14 @@
---
import InlineShared from '../components/InlineShared.astro';
---
<html>
<head>
<title>Testing</title>
</head>
<body>
<InlineShared />
<script>
console.log("page one");
</script>
</body>
</html>

View file

@ -0,0 +1,14 @@
---
import InlineShared from '../components/InlineShared.astro';
---
<html>
<head>
<title>Testing</title>
</head>
<body>
<InlineShared />
<script>
console.log("page two");
</script>
</body>
</html>