chore: set styleLoader.esModule default true (#1581)

web-infra-dev · Feb 19, 2024 · 4180eca · 4180eca
1 parent 252dc38
commit 4180eca
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 22 deletions.
diff --git a/e2e/cases/css/style-loader/.gitignore b/e2e/cases/css/style-loader/.gitignore
@@ -0,0 +1 @@
+test-src/
diff --git a/e2e/cases/css/style-loader/index.test.ts b/e2e/cases/css/style-loader/index.test.ts
@@ -1,19 +1,14 @@
-import { build, gotoPage } from '@e2e/helper';
+import { build, dev, gotoPage, rspackOnlyTest } from '@e2e/helper';
 import { expect, test } from '@playwright/test';
-import { pluginReact } from '@rsbuild/plugin-react';
+import { fse } from '@rsbuild/shared';
+import { join } from 'node:path';
 
 const fixtures = __dirname;
 
 test('should inline style when injectStyles is true', async ({ page }) => {
   const rsbuild = await build({
     cwd: fixtures,
     runServer: true,
-    plugins: [pluginReact()],
-    rsbuildConfig: {
-      output: {
-        injectStyles: true,
-      },
-    },
   });
 
   await gotoPage(page, rsbuild);
@@ -37,8 +32,59 @@ test('should inline style when injectStyles is true', async ({ page }) => {
   await expect(header).toHaveCSS('font-size', '20px');
 
   // less worked
-  const title = page.locator('#header');
+  const title = page.locator('#title');
   await expect(title).toHaveCSS('font-size', '20px');
 
   await rsbuild.close();
 });
+
+rspackOnlyTest(
+  'hmr should work well when injectStyles is true',
+  async ({ page }) => {
+    // HMR cases will fail in Windows
+    if (process.platform === 'win32') {
+      test.skip();
+    }
+
+    await fse.copy(join(fixtures, 'src'), join(fixtures, 'test-src'));
+
+    const rsbuild = await dev({
+      cwd: fixtures,
+      rsbuildConfig: {
+        source: {
+          entry: {
+            index: join(fixtures, 'test-src/index.ts'),
+          },
+        },
+      },
+    });
+
+    await gotoPage(page, rsbuild);
+
+    // scss worked
+    const header = page.locator('#header');
+    await expect(header).toHaveCSS('font-size', '20px');
+
+    // less worked
+    const title = page.locator('#title');
+    await expect(title).toHaveCSS('font-size', '20px');
+
+    const locatorKeep = page.locator('#test-keep');
+    const keepNum = await locatorKeep.innerHTML();
+
+    const filePath = join(fixtures, 'test-src/App.module.less');
+
+    await fse.writeFile(
+      filePath,
+      fse.readFileSync(filePath, 'utf-8').replace('20px', '40px'),
+    );
+
+    // css hmr works well
+    await expect(title).toHaveCSS('font-size', '40px');
+
+    // #test-keep should unchanged when css hmr
+    await expect(locatorKeep.innerHTML()).resolves.toBe(keepNum);
+
+    await rsbuild.close();
+  },
+);
diff --git a/e2e/cases/css/style-loader/rsbuild.config.ts b/e2e/cases/css/style-loader/rsbuild.config.ts
@@ -0,0 +1,9 @@
+import { defineConfig } from '@rsbuild/core';
+import { pluginReact } from '@rsbuild/plugin-react';
+
+export default defineConfig({
+  output: {
+    injectStyles: true,
+  },
+  plugins: [pluginReact()],
+});
diff --git a/e2e/cases/css/style-loader/src/index.ts b/e2e/cases/css/style-loader/src/index.ts
@@ -7,3 +7,11 @@ if (container) {
   const root = createRoot(container);
   root.render(React.createElement(App));
 }
+
+const num = Math.ceil(Math.random() * 100);
+const testEl = document.createElement('div');
+testEl.id = 'test-keep';
+
+testEl.innerHTML = String(num);
+
+document.body.appendChild(testEl);
diff --git a/packages/core/src/provider/plugins/css.ts b/packages/core/src/provider/plugins/css.ts
@@ -58,10 +58,7 @@ export async function applyBaseCSSRule({
 
     if (!isServer && !isWebWorker) {
       const styleLoaderOptions = mergeChainedOptions({
-        defaults: {
-          // todo: hmr does not work while esModule is true
-          esModule: false,
-        },
+        defaults: {},
         options: config.tools.styleLoader,
       });
 

diff --git a/packages/core/tests/__snapshots__/css.test.ts.snap b/packages/core/tests/__snapshots__/css.test.ts.snap
@@ -386,9 +386,6 @@ exports[`plugin-css injectStyles > should use css-loader + style-loader when inj
         "use": [
           {
             "loader": "<ROOT>/packages/shared/compiled/style-loader",
-            "options": {
-              "esModule": false,
-            },
           },
           {
             "loader": "<ROOT>/packages/shared/compiled/css-loader",
@@ -574,9 +571,6 @@ exports[`plugin-less > should add less-loader and css-loader when injectStyles 1
         "use": [
           {
             "loader": "<ROOT>/packages/shared/compiled/style-loader",
-            "options": {
-              "esModule": false,
-            },
           },
           {
             "loader": "<ROOT>/packages/shared/compiled/css-loader",
@@ -1025,9 +1019,6 @@ exports[`plugin-sass > should add sass-loader and css-loader when injectStyles 1
         "use": [
           {
             "loader": "<ROOT>/packages/shared/compiled/style-loader",
-            "options": {
-              "esModule": false,
-            },
           },
           {
             "loader": "<ROOT>/packages/shared/compiled/css-loader",