Skip to content

Commit

Permalink
Fix CSS handling for experimental.directRenderScript (#11026)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed May 13, 2024
1 parent 1e33cd8 commit 8dfb1a2
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-ads-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Skips rendering script tags if it's inlined and empty when `experimental.directRenderScript` is enabled
5 changes: 5 additions & 0 deletions .changeset/rich-melons-worry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes CSS handling if imported in a script tag in an Astro file when `experimental.directRenderScript` is enabled
26 changes: 26 additions & 0 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ export interface BuildInternals {
*/
pagesByClientOnly: Map<string, Set<PageBuildData>>;

/**
* A map for page-specific information by a script in an Astro file
*/
pagesByScriptId: Map<string, Set<PageBuildData>>;

/**
* A map of hydrated components to export names that are discovered during the SSR build.
* These will be used as the top-level entrypoints for the client build.
Expand Down Expand Up @@ -133,6 +138,7 @@ export function createBuildInternals(): BuildInternals {
pageOptionsByPage: new Map(),
pagesByViteID: new Map(),
pagesByClientOnly: new Map(),
pagesByScriptId: new Map(),

propagatedStylesMap: new Map(),
propagatedScriptsMap: new Map(),
Expand Down Expand Up @@ -181,6 +187,26 @@ export function trackClientOnlyPageDatas(
}
}

/**
* Tracks scripts to the pages they are associated with. (experimental.directRenderScript)
*/
export function trackScriptPageDatas(
internals: BuildInternals,
pageData: PageBuildData,
scriptIds: string[]
) {
for (const scriptId of scriptIds) {
let pageDataSet: Set<PageBuildData>;
if (internals.pagesByScriptId.has(scriptId)) {
pageDataSet = internals.pagesByScriptId.get(scriptId)!;
} else {
pageDataSet = new Set<PageBuildData>();
internals.pagesByScriptId.set(scriptId, pageDataSet);
}
pageDataSet.add(pageData);
}
}

export function* getPageDatasByChunk(
internals: BuildInternals,
chunk: Rollup.RenderedChunk
Expand Down
24 changes: 20 additions & 4 deletions packages/astro/src/core/build/plugins/plugin-analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import {
getTopLevelPageModuleInfos,
moduleIsTopLevelPage,
} from '../graph.js';
import { getPageDataByViteID, trackClientOnlyPageDatas } from '../internal.js';
import {
getPageDataByViteID,
trackClientOnlyPageDatas,
trackScriptPageDatas,
} from '../internal.js';
import type { StaticBuildOptions } from '../types.js';

function isPropagatedAsset(id: string) {
Expand Down Expand Up @@ -171,9 +175,21 @@ export function vitePluginAnalyzer(
// each script module is its own entrypoint, so we directly assign each script modules to
// `discoveredScripts` here, which will eventually be passed as inputs of the client build.
if (options.settings.config.experimental.directRenderScript && astro.scripts.length) {
for (let i = 0; i < astro.scripts.length; i++) {
const hid = `${id.replace('/@fs', '')}?astro&type=script&index=${i}&lang.ts`;
internals.discoveredScripts.add(hid);
const scriptIds = astro.scripts.map(
(_, i) => `${id.replace('/@fs', '')}?astro&type=script&index=${i}&lang.ts`
);

// Assign as entrypoints for the client bundle
for (const scriptId of scriptIds) {
internals.discoveredScripts.add(scriptId);
}

// The script may import CSS, so we also have to track the pages that use this script
for (const pageInfo of getTopLevelPageModuleInfos(id, this)) {
const newPageData = getPageDataByViteID(internals, pageInfo.id);
if (!newPageData) continue;

trackScriptPageDatas(internals, newPageData, scriptIds);
}
}
}
Expand Down
21 changes: 15 additions & 6 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,21 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
if (pageData) {
appendCSSToPage(pageData, meta, pagesToCss, depth, order);
}
} else if (
options.target === 'client' &&
internals.hoistedScriptIdToPagesMap.has(pageInfo.id)
) {
for (const pageData of getPageDatasByHoistedScriptId(internals, pageInfo.id)) {
appendCSSToPage(pageData, meta, pagesToCss, -1, order);
} else if (options.target === 'client') {
// For scripts or hoisted scripts, walk parents until you find a page, and add the CSS to that page.
if (buildOptions.settings.config.experimental.directRenderScript) {
const pageDatas = internals.pagesByScriptId.get(pageInfo.id)!;
if (pageDatas) {
for (const pageData of pageDatas) {
appendCSSToPage(pageData, meta, pagesToCss, -1, order);
}
}
} else {
if (internals.hoistedScriptIdToPagesMap.has(pageInfo.id)) {
for (const pageData of getPageDatasByHoistedScriptId(internals, pageInfo.id)) {
appendCSSToPage(pageData, meta, pagesToCss, -1, order);
}
}
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions packages/astro/src/runtime/server/render/script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ export async function renderScript(result: SSRResult, id: string) {
result._metadata.renderedScripts.add(id);

const inlined = result.inlinedScripts.get(id);
if (inlined) {
return markHTMLString(`<script type="module">${inlined}</script>`);
if (inlined != null) {
// The inlined script may actually be empty, so skip rendering it altogether if so
if (inlined) {
return markHTMLString(`<script type="module">${inlined}</script>`);
} else {
return '';
}
}

const resolved = await result.resolve(id);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<html lang="en">
<head>
<script>
import "../styles/script-import-style.css";
</script>
</head>
<body>
<h1>Astro</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
background-color: tomato;
}
10 changes: 10 additions & 0 deletions packages/astro/test/hoisted-imports.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,15 @@ describe('Hoisted Imports', () => {
assert.ok(scripts[0].attribs.src);
assert.ok(scripts[1].attribs.src);
});

it('renders styles if imported from the script', async () => {
const html = await fixture.readFile('/script-import-style/index.html');
const $ = cheerio.load(html);
const styles = $('style');
assert.equal(styles.length, 1);
// There should be no script because it's empty (contains only CSS import)
const scripts = $('scripts');
assert.equal(scripts.length, 0);
});
});
});

0 comments on commit 8dfb1a2

Please sign in to comment.