From 8476f2a293df0ebde6a323a530667cc2ec89ca09 Mon Sep 17 00:00:00 2001
From: Matthew Phillips <matthew@skypack.dev>
Date: Thu, 1 Sep 2022 21:27:26 -0400
Subject: [PATCH] Fix client:only when used with JSX (#4592)

---
 packages/astro/src/core/build/internal.ts           | 13 ++++++++++---
 packages/astro/src/core/path.ts                     |  5 +++++
 packages/astro/test/astro-client-only.test.js       |  6 ++++++
 .../src/components/UsingCSSModules.jsx              | 11 +++++++++++
 .../src/components/styles.module.scss               |  3 +++
 .../astro-client-only/src/pages/css-modules.astro   | 11 +++++++++++
 6 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 packages/astro/test/fixtures/astro-client-only/src/components/UsingCSSModules.jsx
 create mode 100644 packages/astro/test/fixtures/astro-client-only/src/components/styles.module.scss
 create mode 100644 packages/astro/test/fixtures/astro-client-only/src/pages/css-modules.astro

diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
index acc2230cd..0c7c346ae 100644
--- a/packages/astro/src/core/build/internal.ts
+++ b/packages/astro/src/core/build/internal.ts
@@ -1,7 +1,7 @@
 import type { OutputChunk, RenderedChunk } from 'rollup';
 import type { PageBuildData, ViteID } from './types';
 
-import { prependForwardSlash } from '../path.js';
+import { prependForwardSlash, removeFileExtension } from '../path.js';
 import { viteID } from '../util.js';
 
 export interface BuildInternals {
@@ -136,8 +136,15 @@ export function* getPageDatasByClientOnlyID(
 ): Generator<PageBuildData, void, unknown> {
 	const pagesByClientOnly = internals.pagesByClientOnly;
 	if (pagesByClientOnly.size) {
-		const pathname = `/@fs${prependForwardSlash(viteid)}`;
-		const pageBuildDatas = pagesByClientOnly.get(pathname);
+		let pathname = `/@fs${prependForwardSlash(viteid)}`;
+		let pageBuildDatas = pagesByClientOnly.get(viteid);
+		// BUG! The compiler partially resolves .jsx to remove the file extension so we have to check again.
+		// We should probably get rid of all `@fs` usage and always fully resolve via Vite,
+		// but this would be a bigger change.
+		if(!pageBuildDatas) {
+			pathname = `/@fs${prependForwardSlash(removeFileExtension(viteid))}`
+			pageBuildDatas = pagesByClientOnly.get(pathname);
+		}
 		if (pageBuildDatas) {
 			for (const pageData of pageBuildDatas) {
 				yield pageData;
diff --git a/packages/astro/src/core/path.ts b/packages/astro/src/core/path.ts
index 427ce23c6..307283c72 100644
--- a/packages/astro/src/core/path.ts
+++ b/packages/astro/src/core/path.ts
@@ -46,3 +46,8 @@ function isString(path: unknown): path is string {
 export function joinPaths(...paths: (string | undefined)[]) {
 	return paths.filter(isString).map(trimSlashes).join('/');
 }
+
+export function removeFileExtension(path: string) {
+	let idx = path.lastIndexOf('.');
+	return idx === -1 ? path : path.slice(0, idx);
+}
diff --git a/packages/astro/test/astro-client-only.test.js b/packages/astro/test/astro-client-only.test.js
index f3435c1d0..90e4b083d 100644
--- a/packages/astro/test/astro-client-only.test.js
+++ b/packages/astro/test/astro-client-only.test.js
@@ -33,6 +33,12 @@ describe('Client only components', () => {
 		expect(css).to.match(/yellowgreen/, 'Svelte styles are added');
 		expect(css).to.match(/Courier New/, 'Global styles are added');
 	});
+
+	it('Includes CSS from components that use CSS modules', async () => {
+		const html = await fixture.readFile('/css-modules/index.html');
+		const $ = cheerioLoad(html);
+		expect($('link[rel=stylesheet]')).to.have.a.lengthOf(1);
+	});
 });
 
 describe('Client only components subpath', () => {
diff --git a/packages/astro/test/fixtures/astro-client-only/src/components/UsingCSSModules.jsx b/packages/astro/test/fixtures/astro-client-only/src/components/UsingCSSModules.jsx
new file mode 100644
index 000000000..5fda52abb
--- /dev/null
+++ b/packages/astro/test/fixtures/astro-client-only/src/components/UsingCSSModules.jsx
@@ -0,0 +1,11 @@
+import Styles from './styles.module.scss';
+
+const ClientApp = () => {
+  return (
+    <div>
+      <h2 className={Styles.red}>This text should be red</h2>
+    </div>
+  );
+};
+
+export default ClientApp;
diff --git a/packages/astro/test/fixtures/astro-client-only/src/components/styles.module.scss b/packages/astro/test/fixtures/astro-client-only/src/components/styles.module.scss
new file mode 100644
index 000000000..c6dd2c88f
--- /dev/null
+++ b/packages/astro/test/fixtures/astro-client-only/src/components/styles.module.scss
@@ -0,0 +1,3 @@
+.red {
+  color: red;
+}
diff --git a/packages/astro/test/fixtures/astro-client-only/src/pages/css-modules.astro b/packages/astro/test/fixtures/astro-client-only/src/pages/css-modules.astro
new file mode 100644
index 000000000..273c1744c
--- /dev/null
+++ b/packages/astro/test/fixtures/astro-client-only/src/pages/css-modules.astro
@@ -0,0 +1,11 @@
+---
+import UsingCSSModules from '../components/UsingCSSModules.jsx';
+---
+<html>
+	<head>
+		<title>Using CSS modules</title>
+	</head>
+	<body>
+		<UsingCSSModules client:only="react" />
+	</body>
+</html>