From 7d95bd9baaf755239fd7d35e4813861b2dbccf42 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Sep 2023 23:31:15 +0530 Subject: [PATCH] Allow vite to refer to inlined CSS (#8351) * 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 --- .changeset/chatty-walls-happen.md | 5 +++ .changeset/fair-countries-admire.md | 5 +++ .eslintrc.cjs | 2 +- packages/astro/src/core/app/index.ts | 2 -- .../src/core/build/plugins/plugin-css.ts | 4 +-- .../test/css-dangling-references.test.js | 36 +++++++++++++++++++ .../css-dangling-references/astro.config.ts | 8 +++++ .../css-dangling-references/package.json | 11 ++++++ .../DynamicallyImportedComponent1.svelte | 6 ++++ .../DynamicallyImportedComponent2.svelte | 6 ++++ .../src/components/Wrapper.svelte | 15 ++++++++ .../src/pages/glob-import-1.astro | 4 +++ .../src/pages/glob-import-2.astro | 5 +++ .../css-inline-stylesheets/package.json | 2 +- packages/integrations/svelte/src/index.ts | 3 +- pnpm-lock.yaml | 12 +++++++ 16 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 .changeset/chatty-walls-happen.md create mode 100644 .changeset/fair-countries-admire.md create mode 100644 packages/astro/test/css-dangling-references.test.js create mode 100644 packages/astro/test/fixtures/css-dangling-references/astro.config.ts create mode 100644 packages/astro/test/fixtures/css-dangling-references/package.json create mode 100644 packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent1.svelte create mode 100644 packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent2.svelte create mode 100644 packages/astro/test/fixtures/css-dangling-references/src/components/Wrapper.svelte create mode 100644 packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-1.astro create mode 100644 packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-2.astro diff --git a/.changeset/chatty-walls-happen.md b/.changeset/chatty-walls-happen.md new file mode 100644 index 000000000000..58fe5e6edb2c --- /dev/null +++ b/.changeset/chatty-walls-happen.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixed a case where dynamic imports tried to preload inlined stylesheets. diff --git a/.changeset/fair-countries-admire.md b/.changeset/fair-countries-admire.md new file mode 100644 index 000000000000..1868ab016695 --- /dev/null +++ b/.changeset/fair-countries-admire.md @@ -0,0 +1,5 @@ +--- +'@astrojs/svelte': patch +--- + +Removed vite warnings. diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 2e31d2fa76f7..9a6436767e05 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -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', diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index a3af4bcdd5b2..1c0b13148237 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -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 diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts index 87b52cbdb279..b72acd27e81d 100644 --- a/packages/astro/src/core/build/plugins/plugin-css.ts +++ b/packages/astro/src/core/build/plugins/plugin-css.ts @@ -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 || @@ -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 diff --git a/packages/astro/test/css-dangling-references.test.js b/packages/astro/test/css-dangling-references.test.js new file mode 100644 index 000000000000..4f3c6e77d574 --- /dev/null +++ b/packages/astro/test/css-dangling-references.test.js @@ -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) + }) +}) \ No newline at end of file diff --git a/packages/astro/test/fixtures/css-dangling-references/astro.config.ts b/packages/astro/test/fixtures/css-dangling-references/astro.config.ts new file mode 100644 index 000000000000..5c49c2fd0bbd --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/astro.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from 'astro/config'; +import svelte from '@astrojs/svelte'; + +// https://astro.build/config +export default defineConfig({ + integrations: [svelte()], +}); + diff --git a/packages/astro/test/fixtures/css-dangling-references/package.json b/packages/astro/test/fixtures/css-dangling-references/package.json new file mode 100644 index 000000000000..9ac28d282214 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/package.json @@ -0,0 +1,11 @@ +{ + "name": "@test/css-dangling-references", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@astrojs/svelte": "workspace:*", + "svelte": "4" + } + } + \ No newline at end of file diff --git a/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent1.svelte b/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent1.svelte new file mode 100644 index 000000000000..3ac8a03af7e1 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent1.svelte @@ -0,0 +1,6 @@ + +

This sentence should have a gold background.

diff --git a/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent2.svelte b/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent2.svelte new file mode 100644 index 000000000000..7c344f93d480 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/src/components/DynamicallyImportedComponent2.svelte @@ -0,0 +1,6 @@ + +

This sentence should have a lavender background color.

diff --git a/packages/astro/test/fixtures/css-dangling-references/src/components/Wrapper.svelte b/packages/astro/test/fixtures/css-dangling-references/src/components/Wrapper.svelte new file mode 100644 index 000000000000..083040742c15 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/src/components/Wrapper.svelte @@ -0,0 +1,15 @@ + + +{#await AppModule() then Mod} + +{/await} diff --git a/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-1.astro b/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-1.astro new file mode 100644 index 000000000000..847f9fddb828 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-1.astro @@ -0,0 +1,4 @@ +--- +import Wrapper from "../components/Wrapper.svelte" +--- + diff --git a/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-2.astro b/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-2.astro new file mode 100644 index 000000000000..b98c4ef41a24 --- /dev/null +++ b/packages/astro/test/fixtures/css-dangling-references/src/pages/glob-import-2.astro @@ -0,0 +1,5 @@ +--- +import Wrapper from "../components/Wrapper.svelte" +--- + + diff --git a/packages/astro/test/fixtures/css-inline-stylesheets/package.json b/packages/astro/test/fixtures/css-inline-stylesheets/package.json index 0d4a8617da8e..36ae3405054b 100644 --- a/packages/astro/test/fixtures/css-inline-stylesheets/package.json +++ b/packages/astro/test/fixtures/css-inline-stylesheets/package.json @@ -1,5 +1,5 @@ { - "name": "@test/css-inline-stylesheets-always", + "name": "@test/css-inline-stylesheets", "version": "0.0.0", "private": true, "dependencies": { diff --git a/packages/integrations/svelte/src/index.ts b/packages/integrations/svelte/src/index.ts index ab87a59b22d3..a9d4f37c9ac6 100644 --- a/packages/integrations/svelte/src/index.ts +++ b/packages/integrations/svelte/src/index.ts @@ -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 {} } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9efde990009f..43b0c149e3ec 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2437,6 +2437,18 @@ importers: packages/astro/test/fixtures/css-assets/packages/font-awesome: {} + packages/astro/test/fixtures/css-dangling-references: + dependencies: + '@astrojs/svelte': + specifier: workspace:* + version: link:../../../../integrations/svelte + astro: + specifier: workspace:* + version: link:../../.. + svelte: + specifier: '4' + version: 4.2.0 + packages/astro/test/fixtures/css-import-as-inline: dependencies: astro: