From 1187934f1417d0cf4ba68e550cdafec17ec5bc4c Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 17 Nov 2022 16:40:42 -0500 Subject: [PATCH 1/7] Use Vite's resolve to resolve paths for client:only --- packages/astro/src/core/compile/compile.ts | 6 ++++++ packages/astro/src/vite-plugin-astro/index.ts | 16 ++++++++++++++++ packages/astro/test/astro-client-only.test.js | 10 ++++++++++ .../src/components/TSXComponent.tsx | 6 ++++++ .../astro-client-only/src/components/global2.css | 3 +++ .../src/pages/tsx-no-extension.astro | 9 +++++++++ 6 files changed, 50 insertions(+) create mode 100644 packages/astro/test/fixtures/astro-client-only/src/components/TSXComponent.tsx create mode 100644 packages/astro/test/fixtures/astro-client-only/src/components/global2.css create mode 100644 packages/astro/test/fixtures/astro-client-only/src/pages/tsx-no-extension.astro diff --git a/packages/astro/src/core/compile/compile.ts b/packages/astro/src/core/compile/compile.ts index 716c45e864d7..c75ae60459f7 100644 --- a/packages/astro/src/core/compile/compile.ts +++ b/packages/astro/src/core/compile/compile.ts @@ -22,6 +22,7 @@ export interface CompileProps { viteConfig: ResolvedConfig; filename: string; source: string; + resolve: (specifier: string, parent: string) => Promise; } async function compile({ @@ -29,6 +30,7 @@ async function compile({ viteConfig, filename, source, + resolve }: CompileProps): Promise { let cssDeps = new Set(); let cssTransformErrors: AstroError[] = []; @@ -54,6 +56,10 @@ async function compile({ cssTransformErrors, }), async resolvePath(specifier) { + const resolved = await resolve(specifier, filename); + if(resolved) { + return resolved; + } return resolvePath(specifier, filename); }, }) diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index fd9b1805d314..bc94ea065cf3 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -56,6 +56,15 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P return slash(fileURLToPath(resolvedURL)) + resolvedURL.search; } + function makeCompileResolve(ctx: PluginContext) { + return async function(specifier: string, parent: string): Promise { + const resolveResult = await ctx.resolve(specifier, parent); + return resolveResult?.id ?? undefined; + }; + } + + let pluginContext: PluginContext; + return { name: 'astro:build', enforce: 'pre', // run transforms before other plugins can @@ -64,6 +73,10 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P }, // note: don’t claim .astro files with resolveId() — it prevents Vite from transpiling the final JS (import.meta.glob, etc.) async resolveId(id, from, opts) { + // Cache the pluginContext the first time this runs so we can use it in HMR updates + if(pluginContext === undefined) { + pluginContext = this; + } // If resolving from an astro subresource such as a hoisted script, // we need to resolve relative paths ourselves. if (from) { @@ -117,6 +130,7 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P viteConfig: resolvedConfig, filename, source, + resolve: makeCompileResolve(this), }; switch (query.type) { @@ -212,6 +226,7 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P viteConfig: resolvedConfig, filename, source, + resolve: makeCompileResolve(this), }; try { @@ -334,6 +349,7 @@ ${source} viteConfig: resolvedConfig, filename: context.file, source: await context.read(), + resolve: makeCompileResolve(pluginContext), }; const compile = () => cachedCompilation(compileProps); return handleHotUpdate(context, { diff --git a/packages/astro/test/astro-client-only.test.js b/packages/astro/test/astro-client-only.test.js index ef92d80054e8..f3329c683bcf 100644 --- a/packages/astro/test/astro-client-only.test.js +++ b/packages/astro/test/astro-client-only.test.js @@ -93,4 +93,14 @@ describe('Client only components subpath', () => { expect(css).to.match(/yellowgreen/, 'Svelte styles are added'); expect(css).to.match(/Courier New/, 'Global styles are added'); }); + + it('Adds the CSS to the page for TSX components', async () => { + const html = await fixture.readFile('/tsx-no-extension/index.html'); + const $ = cheerioLoad(html); + + const href = $('link[rel=stylesheet]').attr('href'); + const css = await fixture.readFile(href.replace(/\/blog/, '')); + + expect(css).to.match(/purple/, 'Global styles from tsx component are added'); + }); }); diff --git a/packages/astro/test/fixtures/astro-client-only/src/components/TSXComponent.tsx b/packages/astro/test/fixtures/astro-client-only/src/components/TSXComponent.tsx new file mode 100644 index 000000000000..8a8afbde477f --- /dev/null +++ b/packages/astro/test/fixtures/astro-client-only/src/components/TSXComponent.tsx @@ -0,0 +1,6 @@ +import React from 'react'; +import './global2.css'; + +export default function() { + return
i am react
+} diff --git a/packages/astro/test/fixtures/astro-client-only/src/components/global2.css b/packages/astro/test/fixtures/astro-client-only/src/components/global2.css new file mode 100644 index 000000000000..6bfd2ae576a9 --- /dev/null +++ b/packages/astro/test/fixtures/astro-client-only/src/components/global2.css @@ -0,0 +1,3 @@ +body { + color: purple; +} diff --git a/packages/astro/test/fixtures/astro-client-only/src/pages/tsx-no-extension.astro b/packages/astro/test/fixtures/astro-client-only/src/pages/tsx-no-extension.astro new file mode 100644 index 000000000000..a33d71196698 --- /dev/null +++ b/packages/astro/test/fixtures/astro-client-only/src/pages/tsx-no-extension.astro @@ -0,0 +1,9 @@ +--- +import ReactTSXComponent from '../components/TSXComponent'; +--- + +Client only pages + + + + From c1d7d6bea15ef24b42eee4dd4d521b98ad2b0514 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 17 Nov 2022 16:42:05 -0500 Subject: [PATCH 2/7] Adding a changeset --- .changeset/pink-cheetahs-heal.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pink-cheetahs-heal.md diff --git a/.changeset/pink-cheetahs-heal.md b/.changeset/pink-cheetahs-heal.md new file mode 100644 index 000000000000..719f29479a0b --- /dev/null +++ b/.changeset/pink-cheetahs-heal.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Use Vite's resolve to resolve paths for client:only From b616ba7487e8fba18d21383cdf48c677c70a3358 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 17 Nov 2022 16:46:52 -0500 Subject: [PATCH 3/7] Add it to the markdown legacy plugin too --- packages/astro/src/vite-plugin-markdown-legacy/index.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/astro/src/vite-plugin-markdown-legacy/index.ts b/packages/astro/src/vite-plugin-markdown-legacy/index.ts index 3b0e06a0839e..257080bfe22b 100644 --- a/packages/astro/src/vite-plugin-markdown-legacy/index.ts +++ b/packages/astro/src/vite-plugin-markdown-legacy/index.ts @@ -1,3 +1,4 @@ +import type { PluginContext } from 'rollup'; import { renderMarkdown } from '@astrojs/markdown-remark'; import ancestor from 'common-ancestor-path'; import esbuild from 'esbuild'; @@ -64,6 +65,13 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin { return filename; } + function makeCompileResolve(ctx: PluginContext) { + return async function(specifier: string, parent: string): Promise { + const resolveResult = await ctx.resolve(specifier, parent); + return resolveResult?.id ?? undefined; + }; + } + // Weird Vite behavior: Vite seems to use a fake "index.html" importer when you // have `enforce: pre`. This can probably be removed once the vite issue is fixed. // see: https://github.com/vitejs/vite/issues/5981 @@ -218,6 +226,7 @@ ${setup}`.trim(); viteConfig: resolvedConfig, filename, source: astroResult, + resolve: makeCompileResolve(this), }; let transformResult = await cachedCompilation(compileProps); From 7bb3465b99ed162b01bea562252d2918af5482ee Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 18 Nov 2022 08:22:05 -0500 Subject: [PATCH 4/7] Remove fully resolving --- packages/astro/src/core/compile/compile.ts | 6 ------ packages/astro/src/vite-plugin-astro/index.ts | 16 ---------------- .../src/vite-plugin-markdown-legacy/index.ts | 8 -------- 3 files changed, 30 deletions(-) diff --git a/packages/astro/src/core/compile/compile.ts b/packages/astro/src/core/compile/compile.ts index c75ae60459f7..716c45e864d7 100644 --- a/packages/astro/src/core/compile/compile.ts +++ b/packages/astro/src/core/compile/compile.ts @@ -22,7 +22,6 @@ export interface CompileProps { viteConfig: ResolvedConfig; filename: string; source: string; - resolve: (specifier: string, parent: string) => Promise; } async function compile({ @@ -30,7 +29,6 @@ async function compile({ viteConfig, filename, source, - resolve }: CompileProps): Promise { let cssDeps = new Set(); let cssTransformErrors: AstroError[] = []; @@ -56,10 +54,6 @@ async function compile({ cssTransformErrors, }), async resolvePath(specifier) { - const resolved = await resolve(specifier, filename); - if(resolved) { - return resolved; - } return resolvePath(specifier, filename); }, }) diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts index bc94ea065cf3..fd9b1805d314 100644 --- a/packages/astro/src/vite-plugin-astro/index.ts +++ b/packages/astro/src/vite-plugin-astro/index.ts @@ -56,15 +56,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P return slash(fileURLToPath(resolvedURL)) + resolvedURL.search; } - function makeCompileResolve(ctx: PluginContext) { - return async function(specifier: string, parent: string): Promise { - const resolveResult = await ctx.resolve(specifier, parent); - return resolveResult?.id ?? undefined; - }; - } - - let pluginContext: PluginContext; - return { name: 'astro:build', enforce: 'pre', // run transforms before other plugins can @@ -73,10 +64,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P }, // note: don’t claim .astro files with resolveId() — it prevents Vite from transpiling the final JS (import.meta.glob, etc.) async resolveId(id, from, opts) { - // Cache the pluginContext the first time this runs so we can use it in HMR updates - if(pluginContext === undefined) { - pluginContext = this; - } // If resolving from an astro subresource such as a hoisted script, // we need to resolve relative paths ourselves. if (from) { @@ -130,7 +117,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P viteConfig: resolvedConfig, filename, source, - resolve: makeCompileResolve(this), }; switch (query.type) { @@ -226,7 +212,6 @@ export default function astro({ settings, logging }: AstroPluginOptions): vite.P viteConfig: resolvedConfig, filename, source, - resolve: makeCompileResolve(this), }; try { @@ -349,7 +334,6 @@ ${source} viteConfig: resolvedConfig, filename: context.file, source: await context.read(), - resolve: makeCompileResolve(pluginContext), }; const compile = () => cachedCompilation(compileProps); return handleHotUpdate(context, { diff --git a/packages/astro/src/vite-plugin-markdown-legacy/index.ts b/packages/astro/src/vite-plugin-markdown-legacy/index.ts index 257080bfe22b..a2f4da25c6c0 100644 --- a/packages/astro/src/vite-plugin-markdown-legacy/index.ts +++ b/packages/astro/src/vite-plugin-markdown-legacy/index.ts @@ -65,13 +65,6 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin { return filename; } - function makeCompileResolve(ctx: PluginContext) { - return async function(specifier: string, parent: string): Promise { - const resolveResult = await ctx.resolve(specifier, parent); - return resolveResult?.id ?? undefined; - }; - } - // Weird Vite behavior: Vite seems to use a fake "index.html" importer when you // have `enforce: pre`. This can probably be removed once the vite issue is fixed. // see: https://github.com/vitejs/vite/issues/5981 @@ -226,7 +219,6 @@ ${setup}`.trim(); viteConfig: resolvedConfig, filename, source: astroResult, - resolve: makeCompileResolve(this), }; let transformResult = await cachedCompilation(compileProps); From d1afd7099cff94ff850670e41c2fc41d89845972 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 18 Nov 2022 08:22:24 -0500 Subject: [PATCH 5/7] Fully resolve in the analyzer --- .../astro/src/core/build/vite-plugin-analyzer.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/astro/src/core/build/vite-plugin-analyzer.ts b/packages/astro/src/core/build/vite-plugin-analyzer.ts index 65da4fd0f80e..aac7cdfc7292 100644 --- a/packages/astro/src/core/build/vite-plugin-analyzer.ts +++ b/packages/astro/src/core/build/vite-plugin-analyzer.ts @@ -95,13 +95,12 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { const cid = c.resolvedPath ? decodeURI(c.resolvedPath) : c.specifier; internals.discoveredClientOnlyComponents.add(cid); clientOnlys.push(cid); - // Bare module specifiers need to be resolved so that the CSS - // plugin can walk up the graph to find which page they belong to. - if (c.resolvedPath === c.specifier) { - const resolvedId = await this.resolve(c.specifier, id); - if (resolvedId) { - clientOnlys.push(resolvedId.id); - } + + const resolvedId = await this.resolve(c.specifier, id); + if (resolvedId) { + clientOnlys.push(resolvedId.id); + } else if(c.resolvedPath) { + clientOnlys.push(decodeURI(c.resolvedPath)); } } From fa88a202eb55d94b9623692a02e42eec05866ada Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 18 Nov 2022 08:23:11 -0500 Subject: [PATCH 6/7] don't do this twice --- packages/astro/src/core/build/vite-plugin-analyzer.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/astro/src/core/build/vite-plugin-analyzer.ts b/packages/astro/src/core/build/vite-plugin-analyzer.ts index aac7cdfc7292..1d329aebd9e2 100644 --- a/packages/astro/src/core/build/vite-plugin-analyzer.ts +++ b/packages/astro/src/core/build/vite-plugin-analyzer.ts @@ -99,8 +99,6 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin { const resolvedId = await this.resolve(c.specifier, id); if (resolvedId) { clientOnlys.push(resolvedId.id); - } else if(c.resolvedPath) { - clientOnlys.push(decodeURI(c.resolvedPath)); } } From 3ea4582f53225883290a65345b80c684013546b6 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 18 Nov 2022 08:23:45 -0500 Subject: [PATCH 7/7] remove dead code --- packages/astro/src/vite-plugin-markdown-legacy/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/astro/src/vite-plugin-markdown-legacy/index.ts b/packages/astro/src/vite-plugin-markdown-legacy/index.ts index a2f4da25c6c0..3b0e06a0839e 100644 --- a/packages/astro/src/vite-plugin-markdown-legacy/index.ts +++ b/packages/astro/src/vite-plugin-markdown-legacy/index.ts @@ -1,4 +1,3 @@ -import type { PluginContext } from 'rollup'; import { renderMarkdown } from '@astrojs/markdown-remark'; import ancestor from 'common-ancestor-path'; import esbuild from 'esbuild';