Skip to content

Commit

Permalink
Include styles imported by hoisted scripts (#4457)
Browse files Browse the repository at this point in the history
* Include styles imported by hoisted scripts

* Add changeset

* remove unused imports
  • Loading branch information
matthewp committed Aug 25, 2022
1 parent 4671087 commit 9490f0e
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/fast-oranges-collect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Include styles imported by hoisted scripts
11 changes: 9 additions & 2 deletions packages/astro/src/core/build/graph.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { GetModuleInfo, ModuleInfo } from 'rollup';

import { resolvedPagesVirtualModuleId } from '../app/index.js';

// This walks up the dependency graph and yields out each ModuleInfo object.
Expand All @@ -22,14 +23,20 @@ export function* walkParentInfos(
}
}

// Returns true if a module is a top-level page. We determine this based on whether
// it is imported by the top-level virtual module.
export function moduleIsTopLevelPage(info: ModuleInfo): boolean {
return info.importers[0] === resolvedPagesVirtualModuleId;
}

// This function walks the dependency graph, going up until it finds a page component.
// This could be a .astro page or a .md page.
export function* getTopLevelPages(
id: string,
ctx: { getModuleInfo: GetModuleInfo }
ctx: { getModuleInfo: GetModuleInfo },
): Generator<[ModuleInfo, number], void, unknown> {
for (const res of walkParentInfos(id, ctx)) {
if (res[0]?.importers[0] === resolvedPagesVirtualModuleId) {
if (moduleIsTopLevelPage(res[0])) {
yield res;
}
}
Expand Down
28 changes: 20 additions & 8 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { OutputChunk, RenderedChunk } from 'rollup';
import type { OutputChunk, RenderedChunk, ModuleInfo, GetModuleInfo } from 'rollup';
import type { PageBuildData, ViteID } from './types';

import { prependForwardSlash } from '../path.js';
Expand Down Expand Up @@ -62,13 +62,6 @@ export function createBuildInternals(): BuildInternals {
// Pure CSS chunks are chunks that only contain CSS.
// This is all of them, and chunkToReferenceIdMap maps them to a hash id used to find the final file.
const pureCSSChunks = new Set<RenderedChunk>();
const chunkToReferenceIdMap = new Map<string, string>();

// This is a mapping of pathname to the string source of all collected
// inline <style> for a page.
const astroStyleMap = new Map<string, string>();
// This is a virtual JS module that imports all dependent styles for a page.
const astroPageStyleMap = new Map<string, string>();

// These are for tracking hoisted script bundling
const hoistedScriptIdToHoistedMap = new Map<string, Set<string>>();
Expand Down Expand Up @@ -204,3 +197,22 @@ export function sortedCSS(pageData: PageBuildData) {
})
.map(([id]) => id);
}

export function isHoistedScript(internals: BuildInternals, id: string): boolean {
return internals.hoistedScriptIdToPagesMap.has(id);
}

export function* getPageDatasByHoistedScriptId(
internals: BuildInternals,
id: string
): Generator<PageBuildData, void, unknown> {
const set = internals.hoistedScriptIdToPagesMap.get(id);
if(set) {
for(const pageId of set) {
const pageData = getPageDataByComponent(internals, pageId.slice(1));
if(pageData) {
yield pageData;
}
}
}
}
45 changes: 28 additions & 17 deletions packages/astro/src/core/build/vite-plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import npath from 'path';
import { Plugin as VitePlugin, ResolvedConfig } from 'vite';
import { isCSSRequest } from '../render/util.js';
import { relativeToSrcDir } from '../util.js';
import { getTopLevelPages, walkParentInfos } from './graph.js';
import { eachPageData, getPageDataByViteID, getPageDatasByClientOnlyID } from './internal.js';
import { getTopLevelPages, walkParentInfos, moduleIsTopLevelPage } from './graph.js';
import { eachPageData, getPageDataByViteID, getPageDatasByClientOnlyID, getPageDatasByHoistedScriptId, isHoistedScript } from './internal.js';

interface PluginOptions {
internals: BuildInternals;
Expand Down Expand Up @@ -104,6 +104,22 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
importedCss: Set<string>;
};

const appendCSSToPage = (pageData: PageBuildData, meta: ViteMetadata, depth: number) => {
for (const importedCssImport of meta.importedCss) {
// CSS is prioritized based on depth. Shared CSS has a higher depth due to being imported by multiple pages.
// Depth info is used when sorting the links on the page.
if (pageData?.css.has(importedCssImport)) {
// eslint-disable-next-line
const cssInfo = pageData?.css.get(importedCssImport)!;
if (depth < cssInfo.depth) {
cssInfo.depth = depth;
}
} else {
pageData?.css.set(importedCssImport, { depth });
}
}
}

for (const [_, chunk] of Object.entries(bundle)) {
if (chunk.type === 'chunk') {
const c = chunk;
Expand All @@ -127,21 +143,16 @@ 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 [pageInfo, depth] of getTopLevelPages(id, this)) {
const pageViteID = pageInfo.id;
const pageData = getPageDataByViteID(internals, pageViteID);

for (const importedCssImport of meta.importedCss) {
// CSS is prioritized based on depth. Shared CSS has a higher depth due to being imported by multiple pages.
// Depth info is used when sorting the links on the page.
if (pageData?.css.has(importedCssImport)) {
// eslint-disable-next-line
const cssInfo = pageData?.css.get(importedCssImport)!;
if (depth < cssInfo.depth) {
cssInfo.depth = depth;
}
} else {
pageData?.css.set(importedCssImport, { depth });
for (const [pageInfo, depth] of walkParentInfos(id, this)) {
if(moduleIsTopLevelPage(pageInfo)) {
const pageViteID = pageInfo.id;
const pageData = getPageDataByViteID(internals, pageViteID);
if(pageData) {
appendCSSToPage(pageData, meta, depth);
}
} else if(options.target === 'client' && isHoistedScript(internals, pageInfo.id)) {
for(const pageData of getPageDatasByHoistedScriptId(internals, pageInfo.id)) {
appendCSSToPage(pageData, meta, -1);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/test/astro-scripts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ describe('Scripts (hoisted and not)', () => {

expect($('script[type="module"]').length).to.be.greaterThan(0);
});

it('Styles imported by hoisted scripts are included on the page', async () => {
let html = await fixture.readFile('/with-styles/index.html');
let $ = cheerio.load(html);

expect($('link[rel=stylesheet]')).to.have.a.lengthOf(1);
});
});

describe('Dev', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
<script>
import "../styles/one.css";
console.log("This component imports styles.");
</script>
</body>
</html>


3 changes: 3 additions & 0 deletions packages/astro/test/fixtures/astro-scripts/src/styles/one.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
background: rebeccapurple;
}

0 comments on commit 9490f0e

Please sign in to comment.