From 65d17857ce5908f0fcb8adc443def7077e5ab8fc Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 11 Nov 2021 14:35:46 -0500 Subject: [PATCH] Uncomment hoisted scripts (#1743) * Uncomment hoisted scripts * Get hoisted scripts to pass * Adds a changeset --- .changeset/little-dogs-help.md | 5 +++ .../vite-plugin-build-html/extract-assets.ts | 18 +++++++- .../astro/src/vite-plugin-build-html/index.ts | 43 ++++++++++++++++--- packages/astro/test/astro-scripts.test.js | 33 ++++++-------- .../astro-scripts/src/components/Widget.astro | 2 +- .../src/components/Widget2.astro | 2 +- .../src/scripts/another_external.js | 1 + .../astro-scripts/src/scripts/something.js | 1 + 8 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 .changeset/little-dogs-help.md create mode 100644 packages/astro/test/fixtures/astro-scripts/src/scripts/another_external.js create mode 100644 packages/astro/test/fixtures/astro-scripts/src/scripts/something.js diff --git a/.changeset/little-dogs-help.md b/.changeset/little-dogs-help.md new file mode 100644 index 000000000..97f1bbbfa --- /dev/null +++ b/.changeset/little-dogs-help.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes hoisted scripts to be bundled during the build diff --git a/packages/astro/src/vite-plugin-build-html/extract-assets.ts b/packages/astro/src/vite-plugin-build-html/extract-assets.ts index cf230b4ac..33bfbc35e 100644 --- a/packages/astro/src/vite-plugin-build-html/extract-assets.ts +++ b/packages/astro/src/vite-plugin-build-html/extract-assets.ts @@ -1,6 +1,6 @@ import { Document, Element, Node } from 'parse5'; import npath from 'path'; -import { findElements, getTagName, getAttribute, findNodes } from '@web/parse5-utils'; +import { findElements, getTagName, getAttribute, findNodes, hasAttribute } from '@web/parse5-utils'; import adapter from 'parse5/lib/tree-adapters/default.js'; const hashedLinkRels = ['stylesheet', 'preload']; @@ -74,6 +74,18 @@ function isInlineScript(node: Element): boolean { } } +function isExternalScript(node: Element): boolean { + switch (getTagName(node)) { + case 'script': + if (getAttribute(node, 'type') === 'module' && hasAttribute(node, 'src')) { + return true; + } + return false; + default: + return false; + } +} + function isInlineStyle(node: Element): boolean { return getTagName(node) === 'style'; } @@ -182,6 +194,10 @@ export function findInlineScripts(document: Document) { return findElements(document, isInlineScript); } +export function findExternalScripts(document: Document) { + return findElements(document, isExternalScript); +} + export function findInlineStyles(document: Document) { return findElements(document, isInlineStyle); } diff --git a/packages/astro/src/vite-plugin-build-html/index.ts b/packages/astro/src/vite-plugin-build-html/index.ts index 114dcbc48..c40f166ad 100644 --- a/packages/astro/src/vite-plugin-build-html/index.ts +++ b/packages/astro/src/vite-plugin-build-html/index.ts @@ -9,7 +9,7 @@ import * as npath from 'path'; import { promises as fs } from 'fs'; import { getAttribute, hasAttribute, getTagName, insertBefore, remove, createScript, createElement, setAttribute } from '@web/parse5-utils'; import { addRollupInput } from './add-rollup-input.js'; -import { findAssets, findInlineScripts, findInlineStyles, getTextContent, isStylesheetLink } from './extract-assets.js'; +import { findAssets, findExternalScripts, findInlineScripts, findInlineStyles, getTextContent, isStylesheetLink } from './extract-assets.js'; import { render as ssrRender } from '../core/ssr/index.js'; import { getAstroStyleId, getAstroPageStyleId } from '../vite-plugin-build-css/index.js'; import { viteifyPath } from '../core/util.js'; @@ -29,6 +29,7 @@ const isAstroInjectedLink = (node: parse5.Element) => isStylesheetLink(node) && const isBuildableLink = (node: parse5.Element, srcRoot: string) => isAstroInjectedLink(node) || getAttribute(node, 'href')?.startsWith(srcRoot); const isBuildableImage = (node: parse5.Element, srcRoot: string) => getTagName(node) === 'img' && getAttribute(node, 'src')?.startsWith(srcRoot); const hasSrcSet = (node: parse5.Element) => tagsWithSrcSet.has(getTagName(node)) && !!getAttribute(node, 'srcset'); +const isHoistedScript = (node: parse5.Element) => getTagName(node) === 'script' && hasAttribute(node, 'hoist'); interface PluginOptions { astroConfig: AstroConfig; @@ -66,7 +67,7 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin { async options(inputOptions) { const htmlInput: Set = new Set(); - const assetInput: Set = new Set(); // TODO remove? + const assetInput: Set = new Set(); const jsInput: Set = new Set(); for (const [component, pageData] of Object.entries(allPages)) { @@ -107,6 +108,20 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin { } } + for(const script of findExternalScripts(document)) { + if(isHoistedScript(script)) { + debugger; + const astroScript = getAttribute(script, 'astro-script'); + const src = getAttribute(script, 'src'); + if (astroScript) { + const js = `import '${src}';`; + const scriptId = ASTRO_SCRIPT_PREFIX + astroScript; + frontEndImports.push(scriptId); + astroScriptMap.set(scriptId, js); + } + } + } + let styles = ''; for (const node of findInlineStyles(document)) { if (hasAttribute(node, 'astro-style')) { @@ -297,10 +312,29 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin { const bundlePath = '/' + bundleId; // Update scripts - let i = 0; + let pageBundleAdded = false; for (let script of findInlineScripts(document)) { if (getAttribute(script, 'astro-script')) { - if (i === 0) { + if (!pageBundleAdded) { + pageBundleAdded = true; + const relPath = npath.posix.relative(pathname, bundlePath); + insertBefore( + script.parentNode, + createScript({ + type: 'module', + src: relPath, + }), + script + ); + } + remove(script); + } + } + + for (let script of findExternalScripts(document)) { + if (getAttribute(script, 'astro-script')) { + if (!pageBundleAdded) { + pageBundleAdded = true; const relPath = npath.posix.relative(pathname, bundlePath); insertBefore( script.parentNode, @@ -313,7 +347,6 @@ export function rollupPluginAstroBuildHTML(options: PluginOptions): VitePlugin { } remove(script); } - i++; } } diff --git a/packages/astro/test/astro-scripts.test.js b/packages/astro/test/astro-scripts.test.js index 23a972a98..cb7e7fe1d 100644 --- a/packages/astro/test/astro-scripts.test.js +++ b/packages/astro/test/astro-scripts.test.js @@ -1,23 +1,21 @@ -/** - * UNCOMMENT: add Vite external script support import { expect } from 'chai'; import cheerio from 'cheerio'; import path from 'path'; import { loadFixture } from './test-utils.js'; -let fixture; - -before(async () => { - fixture = await loadFixture({ projectRoot: './fixtures/astro-scripts/' }); - await fixture.build(); -}); - describe('Hoisted scripts', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ projectRoot: './fixtures/astro-scripts/' }); + await fixture.build(); + }); + it('Moves external scripts up', async () => { const html = await fixture.readFile('/external/index.html'); const $ = cheerio.load(html); - expect($('head script[type="module"][data-astro="hoist"]')).to.have.lengthOf(2); + expect($('head script[type="module"]:not([src="/regular_script.js"])')).to.have.lengthOf(1); expect($('body script')).to.have.lengthOf(0); }); @@ -25,7 +23,7 @@ describe('Hoisted scripts', () => { const html = await fixture.readFile('/inline/index.html'); const $ = cheerio.load(html); - expect($('head script[type="module"][data-astro="hoist"]')).to.have.lengthOf(1); + expect($('head script[type="module"]')).to.have.lengthOf(1); expect($('body script')).to.have.lengthOf(0); }); @@ -35,7 +33,7 @@ describe('Hoisted scripts', () => { let $ = cheerio.load(inline); // test 1: Just one entry module - assert.equal($('script')).to.have.lengthOf(1); + expect($('script')).to.have.lengthOf(1); // test 2: attr removed expect($('script').attr('data-astro')).to.equal(undefined); @@ -49,19 +47,16 @@ describe('Hoisted scripts', () => { it('External page builds the scripts to a single bundle', async () => { let external = await fixture.readFile('/external/index.html'); - $ = cheerio.load(external); + let $ = cheerio.load(external); // test 1: there are two scripts - assert.equal($('script')).to.have.lengthOf(2); + expect($('script')).to.have.lengthOf(2); let el = $('script').get(1); - entryURL = path.join('external', $(el).attr('src')); - let externalEntryJS = await readFile(entryURL); + let entryURL = path.join('external', $(el).attr('src')); + let externalEntryJS = await fixture.readFile(entryURL); // test 2: the JS exists expect(externalEntryJS).to.be.ok; }); }); -*/ - -it.skip('is skipped', () => {}); diff --git a/packages/astro/test/fixtures/astro-scripts/src/components/Widget.astro b/packages/astro/test/fixtures/astro-scripts/src/components/Widget.astro index 56fff46c4..e56bb2638 100644 --- a/packages/astro/test/fixtures/astro-scripts/src/components/Widget.astro +++ b/packages/astro/test/fixtures/astro-scripts/src/components/Widget.astro @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/packages/astro/test/fixtures/astro-scripts/src/components/Widget2.astro b/packages/astro/test/fixtures/astro-scripts/src/components/Widget2.astro index a87763ef2..f8df03293 100644 --- a/packages/astro/test/fixtures/astro-scripts/src/components/Widget2.astro +++ b/packages/astro/test/fixtures/astro-scripts/src/components/Widget2.astro @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/packages/astro/test/fixtures/astro-scripts/src/scripts/another_external.js b/packages/astro/test/fixtures/astro-scripts/src/scripts/another_external.js new file mode 100644 index 000000000..565da9dbc --- /dev/null +++ b/packages/astro/test/fixtures/astro-scripts/src/scripts/another_external.js @@ -0,0 +1 @@ +console.log('another external'); \ No newline at end of file diff --git a/packages/astro/test/fixtures/astro-scripts/src/scripts/something.js b/packages/astro/test/fixtures/astro-scripts/src/scripts/something.js new file mode 100644 index 000000000..26a087af4 --- /dev/null +++ b/packages/astro/test/fixtures/astro-scripts/src/scripts/something.js @@ -0,0 +1 @@ +console.log('some hoisted script'); \ No newline at end of file