Skip to content

Commit

Permalink
Allow vite to refer to inlined CSS (#8351)
Browse files Browse the repository at this point in the history
* fix(client build): keep inlined stylesheets

* add changesets

* appease linter

* eslint: allow variables beginning with an underscore to be unused

* remove eslint-ignore that's no longer needed

* ready for review
  • Loading branch information
lilnasy authored Sep 5, 2023
1 parent 5126c6a commit 7d95bd9
Show file tree
Hide file tree
Showing 16 changed files with 118 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-walls-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixed a case where dynamic imports tried to preload inlined stylesheets.
5 changes: 5 additions & 0 deletions .changeset/fair-countries-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/svelte': patch
---

Removed vite warnings.
2 changes: 1 addition & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
rules: {
// These off/configured-differently-by-default rules fit well for us
'@typescript-eslint/array-type': ['error', { default: 'array-simple' }],
'@typescript-eslint/no-unused-vars': ['error', { ignoreRestSiblings: true }],
'@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: "^_", ignoreRestSiblings: true }],
'no-only-tests/no-only-tests': 'error',
'@typescript-eslint/no-shadow': ['error'],
'no-console': 'warn',
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ export class App {
}
return pathname;
}
// Disable no-unused-vars to avoid breaking signature change
// eslint-disable-next-line @typescript-eslint/no-unused-vars
match(request: Request, _opts: MatchOptions = {}): RouteData | undefined {
const url = new URL(request.url);
// ignore requests matching public assets
Expand Down
4 changes: 1 addition & 3 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
const inlineConfig = settings.config.build.inlineStylesheets;
const { assetsInlineLimit = 4096 } = settings.config.vite?.build ?? {};

Object.entries(bundle).forEach(([id, stylesheet]) => {
Object.entries(bundle).forEach(([_, stylesheet]) => {
if (
stylesheet.type !== 'asset' ||
stylesheet.name?.endsWith('.css') !== true ||
Expand All @@ -217,8 +217,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
? false
: assetSize <= assetsInlineLimit;

if (toBeInlined) delete bundle[id];

// there should be a single js object for each stylesheet,
// allowing the single reference to be shared and checked for duplicates
const sheet: StylesheetAsset = toBeInlined
Expand Down
36 changes: 36 additions & 0 deletions packages/astro/test/css-dangling-references.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { expect } from 'chai';
import { loadFixture } from './test-utils.js';

const cssAssetReferenceRegExp = /_astro\/[A-Za-z0-9\-]+\.[a0-9a-f]{8}\.css/g

describe("When Vite's preloadModule polyfill is used", async () => {

let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/css-dangling-references/'
});
await fixture.build();
});

it('there are no references to deleted CSS chunks', async () => {

const fileNames = await fixture.readdir('/_astro/')
const filePaths = fileNames.map(filename => '_astro/' + filename)

const expectations =
filePaths
.filter(filePath => filePath.endsWith('js'))
.map(async filePath => {
const contents = await fixture.readFile(filePath)
const cssReferences = contents.match(cssAssetReferenceRegExp)

if (cssReferences === null) return

expect(filePaths).to.contain.members(cssReferences, filePath + ' contains a reference to a deleted css asset: ' + cssReferences)
})

await Promise.all(expectations)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineConfig } from 'astro/config';
import svelte from '@astrojs/svelte';

// https://astro.build/config
export default defineConfig({
integrations: [svelte()],
});

11 changes: 11 additions & 0 deletions packages/astro/test/fixtures/css-dangling-references/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@test/css-dangling-references",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/svelte": "workspace:*",
"svelte": "4"
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<style>
h1 {
background-color: gold;
}
</style>
<h1>This sentence should have a gold background.</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<style>
p {
background-color: lavender;
}
</style>
<p>This sentence should have a lavender background color.</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script>
export let path
const allAppModules = import.meta.glob('./*.svelte')
const AppModule = Object.entries(allAppModules).find(
([key]) => key.includes(path)
)[1]
</script>

{#await AppModule() then Mod}
<Mod.default />
{/await}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
import Wrapper from "../components/Wrapper.svelte"
---
<Wrapper path="1" client:load/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
import Wrapper from "../components/Wrapper.svelte"
---
<Wrapper path="2" client:load/>

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@test/css-inline-stylesheets-always",
"name": "@test/css-inline-stylesheets",
"version": "0.0.0",
"private": true,
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion packages/integrations/svelte/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ async function svelteConfigHasPreprocess(root: URL) {
for (const file of svelteConfigFiles) {
const filePath = fileURLToPath(new URL(file, root));
try {
const config = (await import(filePath)).default;
// Suppress warnings by vite: "The above dynamic import cannot be analyzed by Vite."
const config = (await import(/* @vite-ignore */ filePath)).default;
return !!config.preprocess;
} catch {}
}
Expand Down
12 changes: 12 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7d95bd9

Please sign in to comment.