diff --git a/.changeset/quiet-toys-type.md b/.changeset/quiet-toys-type.md new file mode 100644 index 000000000..fd9560fce --- /dev/null +++ b/.changeset/quiet-toys-type.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Include hoisted scripts inside Astro.glob in dev diff --git a/packages/astro/e2e/shared-component-tests.js b/packages/astro/e2e/shared-component-tests.js index 49736501d..ce66f23b2 100644 --- a/packages/astro/e2e/shared-component-tests.js +++ b/packages/astro/e2e/shared-component-tests.js @@ -117,7 +117,7 @@ export function prepareTestFactory(opts) { original.replace('id="client-idle" {...someProps}', 'id="client-idle" count={5}') ); - await expect(count, 'count prop updated').toHaveText('5'); + await expect(count, 'count prop updated').toHaveText('5', { timeout: 10000 }); await expect(counter, 'component styles persisted').toHaveCSS('display', 'grid'); // Edit the client:only component's slot text diff --git a/packages/astro/src/core/render/dev/css.ts b/packages/astro/src/core/render/dev/css.ts index 657289d36..88866952d 100644 --- a/packages/astro/src/core/render/dev/css.ts +++ b/packages/astro/src/core/render/dev/css.ts @@ -2,14 +2,9 @@ import type * as vite from 'vite'; import path from 'path'; import { RuntimeMode } from '../../../@types/astro.js'; -import { unwrapId, viteID } from '../../util.js'; +import { viteID } from '../../util.js'; import { STYLE_EXTENSIONS } from '../util.js'; - -/** - * List of file extensions signalling we can (and should) SSR ahead-of-time - * See usage below - */ -const fileExtensionsToSSR = new Set(['.md']); +import { crawlGraph } from './vite.js'; /** Given a filePath URL, crawl Vite’s module graph to find all style imports. */ export async function getStylesForURL( @@ -20,69 +15,21 @@ export async function getStylesForURL( const importedCssUrls = new Set(); const importedStylesMap = new Map(); - /** recursively crawl the module graph to get all style files imported by parent id */ - async function crawlCSS(_id: string, isFile: boolean, scanned = new Set()) { - const id = unwrapId(_id); - const importedModules = new Set(); - const moduleEntriesForId = isFile - ? // If isFile = true, then you are at the root of your module import tree. - // The `id` arg is a filepath, so use `getModulesByFile()` to collect all - // nodes for that file. This is needed for advanced imports like Tailwind. - viteServer.moduleGraph.getModulesByFile(id) ?? new Set() - : // Otherwise, you are following an import in the module import tree. - // You are safe to use getModuleById() here because Vite has already - // resolved the correct `id` for you, by creating the import you followed here. - new Set([viteServer.moduleGraph.getModuleById(id)]); - - // Collect all imported modules for the module(s). - for (const entry of moduleEntriesForId) { - // Handle this in case an module entries weren't found for ID - // This seems possible with some virtual IDs (ex: `astro:markdown/*.md`) - if (!entry) { - continue; + for await(const importedModule of crawlGraph(viteServer, viteID(filePath), true)) { + const ext = path.extname(importedModule.url).toLowerCase(); + if (STYLE_EXTENSIONS.has(ext)) { + if ( + mode === 'development' && // only inline in development + typeof importedModule.ssrModule?.default === 'string' // ignore JS module styles + ) { + importedStylesMap.set(importedModule.url, importedModule.ssrModule.default); + } else { + // NOTE: We use the `url` property here. `id` would break Windows. + importedCssUrls.add(importedModule.url); } - if (id === entry.id) { - scanned.add(id); - for (const importedModule of entry.importedModules) { - // some dynamically imported modules are *not* server rendered in time - // to only SSR modules that we can safely transform, we check against - // a list of file extensions based on our built-in vite plugins - if (importedModule.id) { - // use URL to strip special query params like "?content" - const { pathname } = new URL(`file://${importedModule.id}`); - if (fileExtensionsToSSR.has(path.extname(pathname))) { - await viteServer.ssrLoadModule(importedModule.id); - } - } - importedModules.add(importedModule); - } - } - } - - // scan imported modules for CSS imports & add them to our collection. - // Then, crawl that file to follow and scan all deep imports as well. - for (const importedModule of importedModules) { - if (!importedModule.id || scanned.has(importedModule.id)) { - continue; - } - const ext = path.extname(importedModule.url).toLowerCase(); - if (STYLE_EXTENSIONS.has(ext)) { - if ( - mode === 'development' && // only inline in development - typeof importedModule.ssrModule?.default === 'string' // ignore JS module styles - ) { - importedStylesMap.set(importedModule.url, importedModule.ssrModule.default); - } else { - // NOTE: We use the `url` property here. `id` would break Windows. - importedCssUrls.add(importedModule.url); - } - } - await crawlCSS(importedModule.id, false, scanned); } } - // Crawl your import graph for CSS files, populating `importedCssUrls` as a result. - await crawlCSS(viteID(filePath), true); return { urls: importedCssUrls, stylesMap: importedStylesMap, diff --git a/packages/astro/src/core/render/dev/index.ts b/packages/astro/src/core/render/dev/index.ts index bd6b3e9a5..14171140f 100644 --- a/packages/astro/src/core/render/dev/index.ts +++ b/packages/astro/src/core/render/dev/index.ts @@ -14,9 +14,9 @@ import { LogOptions } from '../../logger/core.js'; import { isBuildingToSSR, isPage } from '../../util.js'; import { render as coreRender } from '../core.js'; import { RouteCache } from '../route-cache.js'; -import { createModuleScriptElementWithSrcSet } from '../ssr-element.js'; import { collectMdMetadata } from '../util.js'; import { getStylesForURL } from './css.js'; +import { getScriptsForURL } from './scripts.js'; import { resolveClientDevPath } from './resolve.js'; export interface SSROptions { @@ -108,9 +108,7 @@ export async function render( viteServer, } = ssrOpts; // Add hoisted script tags - const scripts = createModuleScriptElementWithSrcSet( - mod.hasOwnProperty('$$metadata') ? Array.from(mod.$$metadata.hoistedScriptPaths()) : [] - ); + const scripts = await getScriptsForURL(filePath, astroConfig, viteServer); // Inject HMR scripts if (isPage(filePath, astroConfig) && mode === 'development') { diff --git a/packages/astro/src/core/render/dev/scripts.ts b/packages/astro/src/core/render/dev/scripts.ts new file mode 100644 index 000000000..f448c451b --- /dev/null +++ b/packages/astro/src/core/render/dev/scripts.ts @@ -0,0 +1,44 @@ +import type { AstroConfig, SSRElement } from '../../../@types/astro'; +import type { ModuleInfo } from 'rollup'; +import type { PluginMetadata as AstroPluginMetadata } from '../../../vite-plugin-astro/types'; +import vite from 'vite'; +import slash from 'slash'; +import { viteID } from '../../util.js'; +import { createModuleScriptElementWithSrc } from '../ssr-element.js'; +import { crawlGraph } from './vite.js'; +import { fileURLToPath } from 'url'; + +export async function getScriptsForURL( + filePath: URL, + astroConfig: AstroConfig, + viteServer: vite.ViteDevServer, +): Promise> { + const elements = new Set(); + const rootID = viteID(filePath); + let rootProjectFolder = slash(fileURLToPath(astroConfig.root)); + const modInfo = viteServer.pluginContainer.getModuleInfo(rootID); + addHoistedScripts(elements, modInfo, rootProjectFolder); + for await(const moduleNode of crawlGraph(viteServer, rootID, true)) { + const id = moduleNode.id; + if(id) { + const info = viteServer.pluginContainer.getModuleInfo(id); + addHoistedScripts(elements, info, rootProjectFolder); + } + } + + return elements; +} + +function addHoistedScripts(set: Set, info: ModuleInfo | null, rootProjectFolder: string) { + if(!info?.meta?.astro) { + return; + } + + let id = info.id; + const astro = info?.meta?.astro as AstroPluginMetadata['astro']; + for(let i = 0; i < astro.scripts.length; i++) { + const scriptId = `${id}?astro&type=script&index=${i}&lang.ts`; + const element = createModuleScriptElementWithSrc(scriptId); + set.add(element); + } +} diff --git a/packages/astro/src/core/render/dev/vite.ts b/packages/astro/src/core/render/dev/vite.ts new file mode 100644 index 000000000..3af077f0e --- /dev/null +++ b/packages/astro/src/core/render/dev/vite.ts @@ -0,0 +1,68 @@ +import vite from 'vite'; +import npath from 'path'; +import { unwrapId } from '../../util.js'; + +/** + * List of file extensions signalling we can (and should) SSR ahead-of-time + * See usage below + */ + const fileExtensionsToSSR = new Set(['.astro', '.md']); + +/** recursively crawl the module graph to get all style files imported by parent id */ +export async function * crawlGraph( + viteServer: vite.ViteDevServer, + _id: string, + isFile: boolean, + scanned = new Set() +) : AsyncGenerator { + const id = unwrapId(_id); + const importedModules = new Set(); + const moduleEntriesForId = isFile + ? // If isFile = true, then you are at the root of your module import tree. + // The `id` arg is a filepath, so use `getModulesByFile()` to collect all + // nodes for that file. This is needed for advanced imports like Tailwind. + viteServer.moduleGraph.getModulesByFile(id) ?? new Set() + : // Otherwise, you are following an import in the module import tree. + // You are safe to use getModuleById() here because Vite has already + // resolved the correct `id` for you, by creating the import you followed here. + new Set([viteServer.moduleGraph.getModuleById(id)]); + + // Collect all imported modules for the module(s). + for (const entry of moduleEntriesForId) { + // Handle this in case an module entries weren't found for ID + // This seems possible with some virtual IDs (ex: `astro:markdown/*.md`) + if (!entry) { + continue; + } + if (id === entry.id) { + scanned.add(id); + for (const importedModule of entry.importedModules) { + // some dynamically imported modules are *not* server rendered in time + // to only SSR modules that we can safely transform, we check against + // a list of file extensions based on our built-in vite plugins + if (importedModule.id) { + // use URL to strip special query params like "?content" + const { pathname } = new URL(`file://${importedModule.id}`); + if (fileExtensionsToSSR.has(npath.extname(pathname))) { + const mod = viteServer.moduleGraph.getModuleById(importedModule.id); + if(!mod?.ssrModule) { + await viteServer.ssrLoadModule(importedModule.id); + } + } + } + importedModules.add(importedModule); + } + } + } + + // scan imported modules for CSS imports & add them to our collection. + // Then, crawl that file to follow and scan all deep imports as well. + for (const importedModule of importedModules) { + if (!importedModule.id || scanned.has(importedModule.id)) { + continue; + } + + yield importedModule; + yield * crawlGraph(viteServer, importedModule.id, false, scanned); + } +} diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index 8263130fc..4a5bbac6e 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -825,7 +825,7 @@ export function renderHead(result: SSRResult): Promise { const scripts = Array.from(result.scripts) .filter(uniqueElements) .map((script, i) => { - return renderElement('script', script); + return renderElement('script', script, false); }); const links = Array.from(result.links) .filter(uniqueElements) diff --git a/packages/astro/src/runtime/server/metadata.ts b/packages/astro/src/runtime/server/metadata.ts index 3093ee7f1..a92768f5c 100644 --- a/packages/astro/src/runtime/server/metadata.ts +++ b/packages/astro/src/runtime/server/metadata.ts @@ -58,39 +58,6 @@ export class Metadata { return metadata?.componentExport || null; } - *hoistedScriptPaths() { - for (const metadata of this.deepMetadata()) { - let i = 0, - pathname = metadata.mockURL.pathname; - - while (i < metadata.hoisted.length) { - // Strip off the leading "/@fs" added during compilation. - yield `${pathname.replace('/@fs', '')}?astro&type=script&index=${i}&lang.ts`; - i++; - } - } - } - - private *deepMetadata(): Generator { - // Yield self - yield this; - // Keep a Set of metadata objects so we only yield them out once. - const seen = new Set(); - for (const { module: mod } of this.modules) { - if (typeof mod.$$metadata !== 'undefined') { - const md = mod.$$metadata as Metadata; - // Call children deepMetadata() which will yield the child metadata - // and any of its children metadatas - for (const childMetdata of md.deepMetadata()) { - if (!seen.has(childMetdata)) { - seen.add(childMetdata); - yield childMetdata; - } - } - } - } - } - private getComponentMetadata(Component: any): ComponentMetadata | null { if (this.metadataCache.has(Component)) { return this.metadataCache.get(Component)!; diff --git a/packages/astro/test/astro-scripts.test.js b/packages/astro/test/astro-scripts.test.js index dac6a18c7..8bc448150 100644 --- a/packages/astro/test/astro-scripts.test.js +++ b/packages/astro/test/astro-scripts.test.js @@ -4,7 +4,6 @@ import { loadFixture } from './test-utils.js'; describe('Scripts (hoisted and not)', () => { let fixture; - before(async () => { fixture = await loadFixture({ root: './fixtures/astro-scripts/', @@ -14,94 +13,133 @@ describe('Scripts (hoisted and not)', () => { }, }, }); - 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"]:not([src="/regular_script.js"])')).to.have.lengthOf(1); - expect($('body script')).to.have.lengthOf(0); + describe('Build', () => { + before(async () => { + 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"]:not([src="/regular_script.js"])')).to.have.lengthOf(1); + expect($('body script')).to.have.lengthOf(0); + }); + + it('Moves inline scripts up', async () => { + const html = await fixture.readFile('/inline/index.html'); + const $ = cheerio.load(html); + + expect($('head script[type="module"]')).to.have.lengthOf(1); + expect($('body script')).to.have.lengthOf(0); + }); + + it('Inline page builds the scripts to a single bundle', async () => { + // Inline page + let inline = await fixture.readFile('/inline/index.html'); + let $ = cheerio.load(inline); + let $el = $('script'); + + // test 1: Just one entry module + expect($el).to.have.lengthOf(1); + + // test 2: attr removed + expect($el.attr('data-astro')).to.equal(undefined); + + expect($el.attr('src')).to.equal(undefined); + const inlineEntryJS = $el.text(); + + // test 3: the JS exists + expect(inlineEntryJS).to.be.ok; + + // test 4: Inline imported JS is included + expect(inlineEntryJS).to.contain( + 'I AM IMPORTED INLINE', + 'The inline imported JS is included in the bundle' + ); + }); + + 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); + + // test 1: there are two scripts + expect($('script')).to.have.lengthOf(2); + + let el = $('script').get(1); + 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; + }); + + it('External page using non-hoist scripts that are modules are built standalone', async () => { + let external = await fixture.readFile('/external-no-hoist/index.html'); + let $ = cheerio.load(external); + + // test 1: there is 1 scripts + expect($('script')).to.have.lengthOf(1); + + // test 2: inside assets + let entryURL = $('script').attr('src'); + expect(entryURL.includes('assets/')).to.equal(true); + }); + + it('External page using non-hoist scripts that are not modules are built standalone', async () => { + let external = await fixture.readFile('/external-no-hoist-classic/index.html'); + let $ = cheerio.load(external); + + // test 1: there is 1 scripts + expect($('script')).to.have.lengthOf(1); + + // test 2: inside assets + let entryURL = $('script').attr('src'); + expect(entryURL.includes('assets/')).to.equal(true); + }); + + it('Scripts added via Astro.glob are hoisted', async () => { + let glob = await fixture.readFile('/glob/index.html'); + let $ = cheerio.load(glob); + + expect($('script[type="module"]').length).to.be.greaterThan(0); + }); }); - it('Moves inline scripts up', async () => { - const html = await fixture.readFile('/inline/index.html'); - const $ = cheerio.load(html); + describe('Dev', () => { + /** @type {import('./test-utils').DevServer} */ + let devServer; + before(async () => { + devServer = await fixture.startDevServer(); + }); - expect($('head script[type="module"]')).to.have.lengthOf(1); - expect($('body script')).to.have.lengthOf(0); - }); + after(async () => { + await devServer.stop(); + }); - it('Inline page builds the scripts to a single bundle', async () => { - // Inline page - let inline = await fixture.readFile('/inline/index.html'); - let $ = cheerio.load(inline); - let $el = $('script'); + it('Scripts added via Astro.glob are hoisted', async () => { + let res = await fixture.fetch('/glob'); + let html = await res.text(); + let $ = cheerio.load(html); - // test 1: Just one entry module - expect($el).to.have.lengthOf(1); - - // test 2: attr removed - expect($el.attr('data-astro')).to.equal(undefined); - - expect($el.attr('src')).to.equal(undefined); - const inlineEntryJS = $el.text(); - - // test 3: the JS exists - expect(inlineEntryJS).to.be.ok; - - // test 4: Inline imported JS is included - expect(inlineEntryJS).to.contain( - 'I AM IMPORTED INLINE', - 'The inline imported JS is included in the bundle' - ); - }); - - 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); - - // test 1: there are two scripts - expect($('script')).to.have.lengthOf(2); - - let el = $('script').get(1); - 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; - }); - - it('External page using non-hoist scripts that are modules are built standalone', async () => { - let external = await fixture.readFile('/external-no-hoist/index.html'); - let $ = cheerio.load(external); - - // test 1: there is 1 scripts - expect($('script')).to.have.lengthOf(1); - - // test 2: inside assets - let entryURL = $('script').attr('src'); - expect(entryURL.includes('assets/')).to.equal(true); - }); - - it('External page using non-hoist scripts that are not modules are built standalone', async () => { - let external = await fixture.readFile('/external-no-hoist-classic/index.html'); - let $ = cheerio.load(external); - - // test 1: there is 1 scripts - expect($('script')).to.have.lengthOf(1); - - // test 2: inside assets - let entryURL = $('script').attr('src'); - expect(entryURL.includes('assets/')).to.equal(true); + let found = 0; + let moduleScripts = $('[type=module]'); + moduleScripts.each((i, el) => { + if($(el).attr('src').includes('Glob/GlobComponent.astro?astro&type=script&index=0&lang.ts')) { + found++; + } + }) + expect(found).to.equal(1); + }); }); }); diff --git a/packages/astro/test/fixtures/astro-scripts/src/components/Glob/GlobComponent.astro b/packages/astro/test/fixtures/astro-scripts/src/components/Glob/GlobComponent.astro new file mode 100644 index 000000000..dca1588f5 --- /dev/null +++ b/packages/astro/test/fixtures/astro-scripts/src/components/Glob/GlobComponent.astro @@ -0,0 +1,4 @@ + +

My Component

diff --git a/packages/astro/test/fixtures/astro-scripts/src/pages/glob.astro b/packages/astro/test/fixtures/astro-scripts/src/pages/glob.astro new file mode 100644 index 000000000..5d7c77bd5 --- /dev/null +++ b/packages/astro/test/fixtures/astro-scripts/src/pages/glob.astro @@ -0,0 +1,16 @@ +--- +const components = await Astro.glob('/src/components/Glob/*'); +const MyComponent = components[0].default; +--- + + + + + + Astro + + +

My Component is below...

+ + +