From ac3c60d48d5c8bbfb783434cafa75b9ffb7c7a7b Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 26 May 2022 14:00:36 -0400 Subject: [PATCH] Fix VITE_ASSET bug (#3439) * Fix VITE_ASSET bug * Updated test that depended on esbuild output * Fix some more tests * Fix css config and postcss tests * Git client only working * Fix static build test * Update tailwind tests * Fix build * Fix css bundling tests * Updated some more tests for windows * Remove tests that are no longer relevant * Cause it to break * Fix bug and add explanation * Adds a changeset * Inline comments about what the hashing is doing * Update packages/astro/src/vite-plugin-build-css/index.ts Co-authored-by: Nate Moore * Update to the lockfile * Minify css * Update tailwind tests Co-authored-by: Nate Moore --- .changeset/red-knives-sit.md | 7 + packages/astro/package.json | 2 +- packages/astro/src/core/build/internal.ts | 16 +- .../astro/src/vite-plugin-build-css/index.ts | 307 ++++++++---------- packages/astro/test/0-css.test.js | 21 +- packages/astro/test/astro-client-only.test.js | 14 +- .../test/astro-css-bundling-import.test.js | 79 ----- packages/astro/test/config-vite.test.js | 2 +- packages/astro/test/css-assets.test.js | 49 +++ .../astro-css-bundling-import/package.json | 8 - .../src/layouts/BaseLayout.astro | 27 -- .../src/layouts/InlineLayout.astro | 28 -- .../src/layouts/PageLayout.astro | 12 - .../src/pages/page-1.astro | 8 - .../src/pages/page-2.astro | 13 - .../src/pages/page-3.astro | 12 - .../src/styles/page-one.css | 3 - .../src/styles/page-three.css | 3 - .../src/styles/page-two.css | 3 - .../src/styles/site.css | 7 - .../test/fixtures/css-assets/package.json | 9 + .../font-awesome/fontawesome-webfont.woff2 | 19 ++ .../font-awesome/fontawesome-webfont2.woff2 | 19 ++ .../packages/font-awesome/package.json | 4 + .../fixtures/css-assets/src/pages/one.astro | 20 ++ .../fixtures/css-assets/src/pages/two.astro | 22 ++ packages/astro/test/postcss.test.js | 9 +- packages/astro/test/tailwindcss.test.js | 2 +- pnpm-lock.yaml | 31 +- 29 files changed, 336 insertions(+), 420 deletions(-) create mode 100644 .changeset/red-knives-sit.md delete mode 100644 packages/astro/test/astro-css-bundling-import.test.js create mode 100644 packages/astro/test/css-assets.test.js delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/package.json delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/BaseLayout.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/InlineLayout.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-3.astro delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-three.css delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css delete mode 100644 packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css create mode 100644 packages/astro/test/fixtures/css-assets/package.json create mode 100644 packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont.woff2 create mode 100644 packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont2.woff2 create mode 100644 packages/astro/test/fixtures/css-assets/packages/font-awesome/package.json create mode 100644 packages/astro/test/fixtures/css-assets/src/pages/one.astro create mode 100644 packages/astro/test/fixtures/css-assets/src/pages/two.astro diff --git a/.changeset/red-knives-sit.md b/.changeset/red-knives-sit.md new file mode 100644 index 0000000000..de4ad7db99 --- /dev/null +++ b/.changeset/red-knives-sit.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Fixes importing npm packages within CSS + +This change fixes a longstanding bug where the string `VITE_ASSET` was left in CSS when trying to import CSS packages. The fix comes thanks to an upstream Vite feature that allows us to hand off most of the CSS bundling work to Vite. diff --git a/packages/astro/package.json b/packages/astro/package.json index c9d8ff7912..2473ddc680 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -77,7 +77,7 @@ "test:e2e:match": "playwright test -g" }, "dependencies": { - "@astrojs/compiler": "^0.15.0", + "@astrojs/compiler": "^0.15.1", "@astrojs/language-server": "^0.13.4", "@astrojs/markdown-remark": "^0.10.1", "@astrojs/prism": "0.4.1", diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts index e796804fbb..7561e17e42 100644 --- a/packages/astro/src/core/build/internal.ts +++ b/packages/astro/src/core/build/internal.ts @@ -112,19 +112,17 @@ export function* getPageDatasByChunk( } } -export function* getPageDatasByClientOnlyChunk( +export function* getPageDatasByClientOnlyID( internals: BuildInternals, - chunk: RenderedChunk + viteid: ViteID ): Generator { const pagesByClientOnly = internals.pagesByClientOnly; if (pagesByClientOnly.size) { - for (const [modulePath] of Object.entries(chunk.modules)) { - // prepend with `/@fs` to match the path used in the compiler's transform() call - const pathname = `/@fs${prependForwardSlash(modulePath)}`; - if (pagesByClientOnly.has(pathname)) { - for (const pageData of pagesByClientOnly.get(pathname)!) { - yield pageData; - } + const pathname = `/@fs${prependForwardSlash(viteid)}`; + const pageBuildDatas = pagesByClientOnly.get(pathname) + if(pageBuildDatas) { + for(const pageData of pageBuildDatas) { + yield pageData; } } } diff --git a/packages/astro/src/vite-plugin-build-css/index.ts b/packages/astro/src/vite-plugin-build-css/index.ts index 55771ffb8b..02e46c1094 100644 --- a/packages/astro/src/vite-plugin-build-css/index.ts +++ b/packages/astro/src/vite-plugin-build-css/index.ts @@ -1,35 +1,12 @@ import { BuildInternals } from '../core/build/internal'; -import type { ModuleInfo, PluginContext } from 'rollup'; +import type { GetModuleInfo, ModuleInfo } from 'rollup'; +import type { PageBuildData } from '../core/build/types'; -import * as path from 'path'; -import esbuild from 'esbuild'; import { Plugin as VitePlugin } from 'vite'; import { isCSSRequest } from '../core/render/util.js'; -import { - getPageDatasByChunk, - getPageDataByViteID, - hasPageDataByViteID, - getPageDatasByClientOnlyChunk, -} from '../core/build/internal.js'; - -const PLUGIN_NAME = '@astrojs/rollup-plugin-build-css'; - -// This is a virtual module that represents the .astro - - - - - - - - diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/InlineLayout.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/InlineLayout.astro deleted file mode 100644 index f58d50dfde..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/InlineLayout.astro +++ /dev/null @@ -1,28 +0,0 @@ ---- -import "../styles/site.css" - -const {title} = Astro.props; ---- - - - - - - - - {title} - - - - -
- -
- - - diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro deleted file mode 100644 index b1b4514ca0..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/layouts/PageLayout.astro +++ /dev/null @@ -1,12 +0,0 @@ ---- -import BaseLayout from "./BaseLayout.astro" -import "../styles/page-one.css" - -const {title} = Astro.props; ---- - - -
- -
-
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro deleted file mode 100644 index cd20c6b368..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-1.astro +++ /dev/null @@ -1,8 +0,0 @@ ---- -import PageLayout from "../layouts/PageLayout.astro" ---- - - -

Page 1

-

Nothing to see here. Check Page 2

-
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro deleted file mode 100644 index 1df7e43533..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-2.astro +++ /dev/null @@ -1,13 +0,0 @@ ---- -import PageLayout from "../layouts/PageLayout.astro" -import "../styles/page-two.css" ---- - - -

Page 2

-

This text should be green, because we want page-2.css to override site.css

-

This works in the dev-server. However in the prod build, the text is blue. Execute npm run build and then execute npx http-server dist/.

-

We can view the built html at https://github-qoihup--8080.local.webcontainer.io/page-2/. The color there is blue.

- -

If it helps debug the issue, rename the page-1.astro file to page-1.astro.bak. Build the prod site and view it. This time the color is green.

-
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-3.astro b/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-3.astro deleted file mode 100644 index 11863b52c7..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/pages/page-3.astro +++ /dev/null @@ -1,12 +0,0 @@ ---- -import PageLayout from "../layouts/InlineLayout.astro" -import styles from "../styles/page-three.css?raw" -import stylesUrl from "../styles/page-one.css?url" ---- - - - - -

Page 3

-

This text should be purple, because we want the inlined page-3.css to override site.css

-
diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css deleted file mode 100644 index ce7da04630..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-one.css +++ /dev/null @@ -1,3 +0,0 @@ -p { - color: blue; -} diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-three.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-three.css deleted file mode 100644 index ab17cb5cf4..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-three.css +++ /dev/null @@ -1,3 +0,0 @@ -p { - color: purple; -} diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css deleted file mode 100644 index 87002430ab..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/page-two.css +++ /dev/null @@ -1,3 +0,0 @@ -p { - color: green; -} diff --git a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css b/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css deleted file mode 100644 index 47a8192ee5..0000000000 --- a/packages/astro/test/fixtures/astro-css-bundling-import/src/styles/site.css +++ /dev/null @@ -1,7 +0,0 @@ -p { - color: red; -} - -h1 { - outline: 1px solid red; -} diff --git a/packages/astro/test/fixtures/css-assets/package.json b/packages/astro/test/fixtures/css-assets/package.json new file mode 100644 index 0000000000..49471d1b70 --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/astro-sitemap-rss", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@astrojs/test-font-awesome-package": "file:./packages/font-awesome" + } +} diff --git a/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont.woff2 b/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont.woff2 new file mode 100644 index 0000000000..5a9cbc682f --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont.woff2 @@ -0,0 +1,19 @@ +HTTP/2 200 +date: Tue, 24 May 2022 14:15:13 GMT +content-type: font/woff2 +content-length: 77160 +access-control-allow-origin: * +cache-control: public, max-age=31536000 +last-modified: Mon, 24 Oct 2016 21:33:21 GMT +etag: "12d68-1vSMun0Hb7by/Wupk6dbncHsvww" +via: 1.1 fly.io +fly-request-id: 01FDSA73RWTQANDR54ZMWPKC4B +cf-cache-status: HIT +age: 23685787 +accept-ranges: bytes +expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct" +strict-transport-security: max-age=31536000; includeSubDomains; preload +x-content-type-options: nosniff +server: cloudflare +cf-ray: 7106a46819b78005-IAD + diff --git a/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont2.woff2 b/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont2.woff2 new file mode 100644 index 0000000000..a3a72f93f0 --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/packages/font-awesome/fontawesome-webfont2.woff2 @@ -0,0 +1,19 @@ +HTTP/2 200 +date: Tue, 24 May 2022 14:15:13 GMT2 +content-type: font/woff2 +content-length: 77160 +access-control-allow-origin: * +cache-control: public, max-age=31536000 +last-modified: Mon, 24 Oct 2016 21:33:21 GMT +etag: "12d68-1vSMun0Hb7by/Wupk6dbncHsvww" +via: 1.1 fly.io +fly-request-id: 01FDSA73RWTQANDR54ZMWPKC4B +cf-cache-status: HIT +age: 23685787 +accept-ranges: bytes +expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct" +strict-transport-security: max-age=31536000; includeSubDomains; preload +x-content-type-options: nosniff +server: cloudflare +cf-ray: 7106a46819b78005-IAD + diff --git a/packages/astro/test/fixtures/css-assets/packages/font-awesome/package.json b/packages/astro/test/fixtures/css-assets/packages/font-awesome/package.json new file mode 100644 index 0000000000..380c365fdf --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/packages/font-awesome/package.json @@ -0,0 +1,4 @@ +{ + "name": "@astrojs/test-font-awesome-package", + "version": "0.0.1" +} diff --git a/packages/astro/test/fixtures/css-assets/src/pages/one.astro b/packages/astro/test/fixtures/css-assets/src/pages/one.astro new file mode 100644 index 0000000000..a11c659691 --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/src/pages/one.astro @@ -0,0 +1,20 @@ + +Test + + + + + diff --git a/packages/astro/test/fixtures/css-assets/src/pages/two.astro b/packages/astro/test/fixtures/css-assets/src/pages/two.astro new file mode 100644 index 0000000000..70663a24bf --- /dev/null +++ b/packages/astro/test/fixtures/css-assets/src/pages/two.astro @@ -0,0 +1,22 @@ + + + Test2 + + + +

test2

+ + diff --git a/packages/astro/test/postcss.test.js b/packages/astro/test/postcss.test.js index 1697116fd4..4ef4cd936d 100644 --- a/packages/astro/test/postcss.test.js +++ b/packages/astro/test/postcss.test.js @@ -4,7 +4,7 @@ import eol from 'eol'; import { loadFixture } from './test-utils.js'; describe('PostCSS', () => { - const PREFIXED_CSS = `{-webkit-appearance:none;appearance:none}`; + const PREFIXED_CSS = `{-webkit-appearance:none;appearance:none`; let fixture; let bundledCSS; @@ -18,7 +18,8 @@ describe('PostCSS', () => { const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); const bundledCSSHREF = $('link[rel=stylesheet][href^=/assets/]').attr('href'); - bundledCSS = await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/')); + bundledCSS = (await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'))) + .replace(/\s/g, '').replace('/n', ''); }); it('works in Astro page styles', () => { @@ -42,8 +43,8 @@ describe('PostCSS', () => { }); it('ignores CSS in public/', async () => { - const publicCSS = await fixture.readFile('/global.css'); + const publicCSS = (await fixture.readFile('/global.css')).trim().replace(/\s/g, '').replace('/n', ''); // neither minified nor prefixed - expect(eol.lf(publicCSS.trim())).to.equal(`.global {\n appearance: none;\n}`); + expect(eol.lf(publicCSS)).to.equal(`.global{appearance:none;}`); }); }); diff --git a/packages/astro/test/tailwindcss.test.js b/packages/astro/test/tailwindcss.test.js index 2facca9e4c..4fa3973c39 100644 --- a/packages/astro/test/tailwindcss.test.js +++ b/packages/astro/test/tailwindcss.test.js @@ -39,7 +39,7 @@ describe('Tailwind', () => { // tailwind escapes brackets, `font-[900]` compiles to `font-\[900\]` expect(bundledCSS, 'supports arbitrary value classes').to.match( - /\.font-\\\[900\\\]{font-weight:900}/ + /\.font-\\\[900\\\]{/ ); // custom theme colors were included diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7c17968c80..00968fda85 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -459,7 +459,7 @@ importers: packages/astro: specifiers: - '@astrojs/compiler': ^0.15.0 + '@astrojs/compiler': ^0.15.1 '@astrojs/language-server': ^0.13.4 '@astrojs/markdown-remark': ^0.10.1 '@astrojs/prism': 0.4.1 @@ -545,7 +545,7 @@ importers: yargs-parser: ^21.0.1 zod: ^3.17.3 dependencies: - '@astrojs/compiler': 0.15.0 + '@astrojs/compiler': 0.15.1 '@astrojs/language-server': 0.13.4 '@astrojs/markdown-remark': link:../markdown/remark '@astrojs/prism': link:../astro-prism @@ -873,12 +873,6 @@ importers: dependencies: astro: link:../../.. - packages/astro/test/fixtures/astro-css-bundling-import: - specifiers: - astro: workspace:* - dependencies: - astro: link:../../.. - packages/astro/test/fixtures/astro-css-bundling-nested-layouts: specifiers: astro: workspace:* @@ -1117,6 +1111,17 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/css-assets: + specifiers: + '@astrojs/test-font-awesome-package': file:./packages/font-awesome + astro: workspace:* + dependencies: + '@astrojs/test-font-awesome-package': file:packages/astro/test/fixtures/css-assets/packages/font-awesome + astro: link:../../.. + + packages/astro/test/fixtures/css-assets/packages/font-awesome: + specifiers: {} + packages/astro/test/fixtures/custom-404: specifiers: astro: workspace:* @@ -2059,8 +2064,8 @@ packages: leven: 3.1.0 dev: true - /@astrojs/compiler/0.15.0: - resolution: {integrity: sha512-gsw9OP/mfMIrdbrBaG5BpP98sWiAymeeVJTi365OwzDvOaePUzKqAMGCQd07RAfeX9eQzTkjDeJJZkIoyt575w==} + /@astrojs/compiler/0.15.1: + resolution: {integrity: sha512-4PcWD7lfX1c6J9GwaVVbQjrMQEtqx3jXRGj92TCIc0EQ6kRKxLymdzRS9eW8yCsgJp6Ym4nCzQ8ifTSVSphjDQ==} dependencies: tsm: 2.2.1 uvu: 0.5.3 @@ -13807,3 +13812,9 @@ packages: /zwitch/2.0.2: resolution: {integrity: sha512-JZxotl7SxAJH0j7dN4pxsTV6ZLXoLdGME+PsjkL/DaBrVryK9kTGq06GfKrwcSOqypP+fdXGoCHE36b99fWVoA==} + + file:packages/astro/test/fixtures/css-assets/packages/font-awesome: + resolution: {directory: packages/astro/test/fixtures/css-assets/packages/font-awesome, type: directory} + name: '@astrojs/test-font-awesome-package' + version: 0.0.1 + dev: false