Uncomment hoisted scripts (#1743)

* Uncomment hoisted scripts

* Get hoisted scripts to pass

* Adds a changeset

.changeset/little-dogs-help.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes hoisted scripts to be bundled during the build

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

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<string> = new Set();
-      const assetInput: Set<string> = new Set(); // TODO remove?
+      const assetInput: Set<string> = new Set();
       const jsInput: Set<string> = 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++;
           }
         }
 

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', () => {});

packages/astro/test/fixtures/astro-scripts/src/components/Widget.astro

@@ -1 +1 @@
-<script hoist type="module" src="/something.js"></script>
+<script hoist type="module" src={Astro.resolve("../scripts/something.js")}></script>

packages/astro/test/fixtures/astro-scripts/src/components/Widget2.astro

@@ -1 +1 @@
-<script hoist type="module" src="/another_external.js"></script>
+<script hoist type="module" src={Astro.resolve("../scripts/another_external.js")}></script>

packages/astro/test/fixtures/astro-scripts/src/scripts/another_external.js

@@ -0,0 +1 @@
+console.log('another external');

packages/astro/test/fixtures/astro-scripts/src/scripts/something.js

@@ -0,0 +1 @@
+console.log('some hoisted script');