Skip to content

Commit

Permalink
Merge branch 'main' into content-layer
Browse files Browse the repository at this point in the history
  • Loading branch information
ascorbic authored Jul 22, 2024
2 parents da24013 + dfbca06 commit 15b74c4
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-pillows-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Excludes hoisted scripts and styles from Astro components imported with `?url` or `?raw`
12 changes: 12 additions & 0 deletions packages/astro/src/vite-plugin-astro-server/vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import npath from 'node:path';
import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../core/constants.js';
import type { ModuleLoader, ModuleNode } from '../core/module-loader/index.js';
import { unwrapId } from '../core/util.js';
import { hasSpecialQueries } from '../vite-plugin-utils/index.js';
import { isCSSRequest } from './util.js';

/**
Expand Down Expand Up @@ -42,6 +43,10 @@ export async function* crawlGraph(
if (id === entry.id) {
scanned.add(id);

// NOTE: It may be worth revisiting if we can crawl direct imports of the module since
// `.importedModules` would also include modules that are dynamically watched, not imported.
// That way we no longer need the below `continue` skips.

// CSS requests `importedModules` are usually from `@import`, but we don't really need
// to crawl into those as the `@import` code are already inlined into this `id`.
// If CSS requests `importedModules` contain non-CSS files, e.g. Tailwind might add HMR
Expand All @@ -50,6 +55,13 @@ export async function* crawlGraph(
if (isCSSRequest(id)) {
continue;
}
// Some special Vite queries like `?url` or `?raw` are known to be a simple default export
// and doesn't have any imports to crawl. However, since they would `this.addWatchFile` the
// underlying module, our logic would crawl into them anyways which is incorrect as they
// don't take part in the final rendering, so we skip it here.
if (hasSpecialQueries(id)) {
continue;
}

for (const importedModule of entry.importedModules) {
if (!importedModule.id) continue;
Expand Down
18 changes: 16 additions & 2 deletions packages/astro/test/astro-basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,14 @@ describe('Astro basic build', () => {
it('Handles importing .astro?raw correctly', async () => {
const html = await fixture.readFile('/import-queries/raw/index.html');
const $ = cheerio.load(html);
assert.equal($('.raw-value').text(), '<h1>Hello</h1>\n');
const rawValue = $('.raw-value').text();
assert.match(rawValue, /<h1>Hello<\/h1>/);
assert.match(rawValue, /<script>/);
assert.match(rawValue, /<style>/);
// The rest of HTML should not contain any scripts or styles hoisted from the raw import
const otherHtml = html.replace(rawValue, '');
assert.doesNotMatch(otherHtml, /<script/);
assert.doesNotMatch(otherHtml, /<style/);
});

describe('preview', () => {
Expand Down Expand Up @@ -223,6 +230,13 @@ describe('Astro basic development', () => {
assert.equal(res.status, 200);
const html = await res.text();
const $ = cheerio.load(html);
assert.equal($('.raw-value').text(), '<h1>Hello</h1>\n');
const rawValue = $('.raw-value').text();
assert.match(rawValue, /<h1>Hello<\/h1>/);
assert.match(rawValue, /<script>/);
assert.match(rawValue, /<style>/);
// The rest of HTML should not contain any scripts or styles hoisted from the raw import.
// However we don't check them here as dev plugins could add scripts and styles dynam
assert.doesNotMatch(html, /_content.astro\?astro&type=style/);
assert.doesNotMatch(html, /_content.astro\?astro&type=script/);
});
});
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
<h1>Hello</h1>
<script>console.log('Should not log')</script>
<style>h1 { color: red; }</style>

0 comments on commit 15b74c4

Please sign in to comment.