From 82a1063cc26cf6d7012d40025a6ad14812c74d1d Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 5 Aug 2022 09:52:12 -0400 Subject: [PATCH] Include CSS when child component uses Astro.glob on page (#4156) * Include CSS when child component uses Astro.glob on page * windows compat * Make work on windows * Remove unnecessary concat --- .changeset/long-toys-rule.md | 5 ++ packages/astro/src/core/build/graph.ts | 7 +-- packages/astro/src/core/build/static-build.ts | 4 +- .../src/core/build/vite-plugin-analyzer.ts | 7 ++- .../build/vite-plugin-css.ts} | 61 +++++++++++++------ packages/astro/src/core/util.ts | 10 +++ .../src/vite-plugin-build-css/resolve.ts | 28 --------- .../src/components/Footer.astro | 12 ++++ .../src/components/Navbar.astro | 6 ++ .../glob-pages-css/src/pages/index.astro | 14 +++++ .../glob-pages-css/src/pages/two.astro | 8 +++ packages/astro/test/glob-pages-css.test.js | 22 +++++++ 12 files changed, 131 insertions(+), 53 deletions(-) create mode 100644 .changeset/long-toys-rule.md rename packages/astro/src/{vite-plugin-build-css/index.ts => core/build/vite-plugin-css.ts} (74%) delete mode 100644 packages/astro/src/vite-plugin-build-css/resolve.ts create mode 100644 packages/astro/test/fixtures/glob-pages-css/src/components/Footer.astro create mode 100644 packages/astro/test/fixtures/glob-pages-css/src/components/Navbar.astro create mode 100644 packages/astro/test/fixtures/glob-pages-css/src/pages/index.astro create mode 100644 packages/astro/test/fixtures/glob-pages-css/src/pages/two.astro create mode 100644 packages/astro/test/glob-pages-css.test.js diff --git a/.changeset/long-toys-rule.md b/.changeset/long-toys-rule.md new file mode 100644 index 000000000000..81580b3b74f8 --- /dev/null +++ b/.changeset/long-toys-rule.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Add CSS to page when child component uses Astro.glob diff --git a/packages/astro/src/core/build/graph.ts b/packages/astro/src/core/build/graph.ts index a0fa4a8b963e..e112845ab300 100644 --- a/packages/astro/src/core/build/graph.ts +++ b/packages/astro/src/core/build/graph.ts @@ -26,11 +26,10 @@ export function* walkParentInfos( export function* getTopLevelPages( id: string, ctx: { getModuleInfo: GetModuleInfo } -): Generator { +): Generator { for (const info of walkParentInfos(id, ctx)) { - const importers = (info?.importers || []).concat(info?.dynamicImporters || []); - if (importers.length <= 2 && importers[0] === resolvedPagesVirtualModuleId) { - yield info.id; + if (info?.importers[0] === resolvedPagesVirtualModuleId) { + yield info; } } } diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index a94174ad7b79..c2e33432cbf1 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -7,7 +7,7 @@ import { BuildInternals, createBuildInternals } from '../../core/build/internal. import { prependForwardSlash } from '../../core/path.js'; import { emptyDir, isModeServerWithNoAdapter, removeDir } from '../../core/util.js'; import { runHookBuildSetup } from '../../integrations/index.js'; -import { rollupPluginAstroBuildCSS } from '../../vite-plugin-build-css/index.js'; +import { rollupPluginAstroBuildCSS } from './vite-plugin-css.js'; import { PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js'; import type { ViteConfigWithSSR } from '../create-vite'; import { info } from '../logger/core.js'; @@ -151,6 +151,7 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp vitePluginInternals(input, internals), vitePluginPages(opts, internals), rollupPluginAstroBuildCSS({ + buildOptions: opts, internals, target: 'server', astroConfig, @@ -233,6 +234,7 @@ async function clientBuild( vitePluginInternals(input, internals), vitePluginHoistedScripts(astroConfig, internals), rollupPluginAstroBuildCSS({ + buildOptions: opts, internals, target: 'client', astroConfig, diff --git a/packages/astro/src/core/build/vite-plugin-analyzer.ts b/packages/astro/src/core/build/vite-plugin-analyzer.ts index ed92dfe5fa4e..a859bcd7c807 100644 --- a/packages/astro/src/core/build/vite-plugin-analyzer.ts +++ b/packages/astro/src/core/build/vite-plugin-analyzer.ts @@ -22,7 +22,8 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { } if (hoistedScripts.size) { - for (const pageId of getTopLevelPages(from, this)) { + for (const pageInfo of getTopLevelPages(from, this)) { + const pageId = pageInfo.id; for (const hid of hoistedScripts) { if (pageScripts.has(pageId)) { pageScripts.get(pageId)?.add(hid); @@ -97,8 +98,8 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { clientOnlys.push(cid); } - for (const pageId of getTopLevelPages(id, this)) { - const pageData = getPageDataByViteID(internals, pageId); + for (const pageInfo of getTopLevelPages(id, this)) { + const pageData = getPageDataByViteID(internals, pageInfo.id); if (!pageData) continue; trackClientOnlyPageDatas(internals, pageData, clientOnlys); diff --git a/packages/astro/src/vite-plugin-build-css/index.ts b/packages/astro/src/core/build/vite-plugin-css.ts similarity index 74% rename from packages/astro/src/vite-plugin-build-css/index.ts rename to packages/astro/src/core/build/vite-plugin-css.ts index 5d9edd66f0f9..87708181cfad 100644 --- a/packages/astro/src/vite-plugin-build-css/index.ts +++ b/packages/astro/src/core/build/vite-plugin-css.ts @@ -1,29 +1,58 @@ import type { GetModuleInfo, OutputChunk } from 'rollup'; -import { BuildInternals } from '../core/build/internal'; -import type { PageBuildData } from '../core/build/types'; +import type { BuildInternals } from './internal'; +import type { PageBuildData, StaticBuildOptions } from './types'; +import type { AstroConfig } from '../../@types/astro'; import crypto from 'crypto'; import esbuild from 'esbuild'; +import npath from 'path'; import { Plugin as VitePlugin } from 'vite'; -import { AstroConfig } from '../@types/astro'; -import { getTopLevelPages, walkParentInfos } from '../core/build/graph.js'; -import { getPageDataByViteID, getPageDatasByClientOnlyID } from '../core/build/internal.js'; -import { isCSSRequest } from '../core/render/util.js'; +import { getTopLevelPages, walkParentInfos } from './graph.js'; +import { getPageDataByViteID, getPageDatasByClientOnlyID } from './internal.js'; +import { relativeToSrcDir } from '../util.js'; +import { isCSSRequest } from '../render/util.js'; interface PluginOptions { internals: BuildInternals; + buildOptions: StaticBuildOptions; target: 'client' | 'server'; astroConfig: AstroConfig; } +// Arbitrary magic number, can change. +const MAX_NAME_LENGTH = 70; + export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] { - const { internals } = options; + const { internals, buildOptions } = options; + const { astroConfig } = buildOptions; + + // Turn a page location into a name to be used for the CSS file. + function nameifyPage(id: string) { + let rel = relativeToSrcDir(astroConfig, id); + // Remove pages, ex. blog/posts/something.astro + if(rel.startsWith('pages/')) { + rel = rel.slice(6); + } + // Remove extension, ex. blog/posts/something + const ext = npath.extname(rel); + const noext = rel.slice(0, rel.length - ext.length); + // Replace slashes with dashes, ex. blog-posts-something + const named = noext.replace(/\//g, '-'); + return named; + } - function createHashOfPageParents(id: string, ctx: { getModuleInfo: GetModuleInfo }): string { + function createNameForParentPages(id: string, ctx: { getModuleInfo: GetModuleInfo }): string { const parents = Array.from(getTopLevelPages(id, ctx)).sort(); - const hash = crypto.createHash('sha256'); + const proposedName = parents.map(page => nameifyPage(page.id)).join('-'); + + // We don't want absurdedly long chunk names, so if this is too long create a hashed version instead. + if(proposedName.length <= MAX_NAME_LENGTH) { + return proposedName; + } + + const hash = crypto.createHash('sha256'); for (const page of parents) { - hash.update(page, 'utf-8'); + hash.update(page.id, 'utf-8'); } return hash.digest('hex').slice(0, 8); } @@ -37,12 +66,9 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] } } - const CSS_PLUGIN_NAME = '@astrojs/rollup-plugin-build-css'; - const CSS_MINIFY_PLUGIN_NAME = '@astrojs/rollup-plugin-build-css-minify'; - return [ { - name: CSS_PLUGIN_NAME, + name: 'astro:rollup-plugin-build-css', outputOptions(outputOptions) { const manualChunks = outputOptions.manualChunks || Function.prototype; @@ -62,7 +88,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] // For CSS, create a hash of all of the pages that use it. // This causes CSS to be built into shared chunks when used by multiple pages. if (isCSSRequest(id)) { - return createHashOfPageParents(id, args[0]); + return createNameForParentPages(id, args[0]); } }; }, @@ -96,7 +122,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] // For this CSS chunk, walk parents until you find a page. Add the CSS to that page. for (const [id] of Object.entries(c.modules)) { - for (const pageViteID of getTopLevelPages(id, this)) { + for (const pageInfo of getTopLevelPages(id, this)) { + const pageViteID = pageInfo.id; const pageData = getPageDataByViteID(internals, pageViteID); for (const importedCssImport of meta.importedCss) { pageData?.css.add(importedCssImport); @@ -110,7 +137,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] }, }, { - name: CSS_MINIFY_PLUGIN_NAME, + name: 'astro:rollup-plugin-build-css-minify', enforce: 'post', async generateBundle(_outputOptions, bundle) { // Minify CSS in each bundle ourselves, since server builds are not minified diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index 4ba0a7286649..ac36f7a05f50 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -186,6 +186,16 @@ export function isModeServerWithNoAdapter(config: AstroConfig): boolean { return config.output === 'server' && !config._ctx.adapter; } +export function relativeToSrcDir(config: AstroConfig, idOrUrl: URL | string) { + let id: string; + if(typeof idOrUrl !== 'string') { + id = unwrapId(viteID(idOrUrl)); + } else { + id = idOrUrl; + } + return id.slice(slash(fileURLToPath(config.srcDir)).length); +} + export function emoji(char: string, fallback: string) { return process.platform !== 'win32' ? char : fallback; } diff --git a/packages/astro/src/vite-plugin-build-css/resolve.ts b/packages/astro/src/vite-plugin-build-css/resolve.ts deleted file mode 100644 index 4bb09d1742ff..000000000000 --- a/packages/astro/src/vite-plugin-build-css/resolve.ts +++ /dev/null @@ -1,28 +0,0 @@ -import type { LoadHook, ResolveIdHook } from 'rollup'; -import type { Plugin as VitePlugin, ResolvedConfig } from 'vite'; - -export function getVitePluginByName(viteConfig: ResolvedConfig, pluginName: string): VitePlugin { - const plugin = viteConfig.plugins.find(({ name }) => name === pluginName); - if (!plugin) throw new Error(`${pluginName} plugin couldn’t be found`); - return plugin; -} - -export function getViteResolvePlugin(viteConfig: ResolvedConfig): VitePlugin { - return getVitePluginByName(viteConfig, 'vite:resolve'); -} - -export function getViteLoadFallbackPlugin(viteConfig: ResolvedConfig): VitePlugin { - return getVitePluginByName(viteConfig, 'vite:load-fallback'); -} - -export function getViteResolve(viteConfig: ResolvedConfig): ResolveIdHook { - const plugin = getViteResolvePlugin(viteConfig); - if (!plugin.resolveId) throw new Error(`vite:resolve has no resolveId() hook`); - return plugin.resolveId.bind(null as any) as any; -} - -export function getViteLoad(viteConfig: ResolvedConfig): LoadHook { - const plugin = getViteLoadFallbackPlugin(viteConfig); - if (!plugin.load) throw new Error(`vite:load-fallback has no load() hook`); - return plugin.load.bind(null as any) as any; -} diff --git a/packages/astro/test/fixtures/glob-pages-css/src/components/Footer.astro b/packages/astro/test/fixtures/glob-pages-css/src/components/Footer.astro new file mode 100644 index 000000000000..32d8844f24be --- /dev/null +++ b/packages/astro/test/fixtures/glob-pages-css/src/components/Footer.astro @@ -0,0 +1,12 @@ +--- +// The side-effect of this happening is needed for the test. +await Astro.glob("../pages/**/*.astro"); +--- + +
diff --git a/packages/astro/test/fixtures/glob-pages-css/src/components/Navbar.astro b/packages/astro/test/fixtures/glob-pages-css/src/components/Navbar.astro new file mode 100644 index 000000000000..65168efce7f7 --- /dev/null +++ b/packages/astro/test/fixtures/glob-pages-css/src/components/Navbar.astro @@ -0,0 +1,6 @@ +--- +// The side-effect of this happening is needed for the test. +const pages = await Astro.glob("../pages/**/*.astro"); +--- + +
navbar
diff --git a/packages/astro/test/fixtures/glob-pages-css/src/pages/index.astro b/packages/astro/test/fixtures/glob-pages-css/src/pages/index.astro new file mode 100644 index 000000000000..8a5f12c5f480 --- /dev/null +++ b/packages/astro/test/fixtures/glob-pages-css/src/pages/index.astro @@ -0,0 +1,14 @@ +--- +import Footer from '../components/Footer.astro'; +import Navbar from '../components/Navbar.astro'; +--- + + + Testing + + + +

Testing

+