Merge pull request #513 from dilanx:postcss-fix

Fixed issue where postcss plugins weren't being applied
dilanx · May 26, 2023 · f451908 · f451908
2 parents 6c17b95 + e56698d
commit f451908
Show file tree

Hide file tree

Showing 13 changed files with 994 additions and 41 deletions.
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -1,10 +1,6 @@
 name: tests
 
-on:
-  push:
-    branches: [main]
-  pull_request:
-    branches: [main]
+on: [push, pull_request]
 
 jobs:
   build:
@@ -22,7 +18,7 @@ jobs:
         with:
           node-version: ${{ matrix.node-version }}
       - run: npm ci
-      - run: CI=false npm run build
+      - run: npm run build
       - run: npm run test:unit
       - run: npx playwright install-deps
-      - run: CI=false npm run test:integration
+      - run: NODE_ENV=production npm run test:integration
diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
   ],
   "scripts": {
     "test:unit": "jest --config test/unit/jest.config.js",
-    "test:integration": "jest --config test/integration/jest.config.js --runInBand",
+    "test:integration": "NODE_ENV=production jest --config test/integration/jest.config.js --runInBand",
     "lint": "npm run lint:ts && npm run lint:es",
     "lint:ts": "tsc --noEmit",
     "lint:es": "eslint --ext .ts",

diff --git a/packages/craco/src/lib/features/webpack/style/postcss.ts b/packages/craco/src/lib/features/webpack/style/postcss.ts
@@ -83,10 +83,10 @@ function extendsPostcss(
 
     if (match.loader.options && !isString(match.loader.options)) {
       if (match.loader.options.postcssOptions) {
-        match.loader.options.postcssOptions.plugins = () => postcssPlugins;
+        match.loader.options.postcssOptions.plugins = postcssPlugins;
       } else {
         match.loader.options.postcssOptions = {
-          plugins: () => postcssPlugins,
+          plugins: postcssPlugins,
         };
       }
     }

diff --git a/test/integration/fixtures/autoprefixer-test/index.test.js b/test/integration/fixtures/autoprefixer-test/index.test.js
@@ -0,0 +1,25 @@
+'use strict';
+const path = require('path');
+const fs = require('fs');
+
+test('Should have the expected styles', async () => {
+  const expectedStyles = "::-webkit-input-placeholder{color:gray}::placeholder{color:gray}.image{background-image:url(https://google.com)}@media (-webkit-min-device-pixel-ratio:2),(min-resolution:2dppx){.image{background-image:url(http://nxetflix.com)}}";
+
+  //we check if the compiled css is what we expect for autoprefixer
+  const prom = new Promise((resolve, reject) => {
+    fs.readdir('./test/integration/fixtures/autoprefixer-test/test-project/build/static/css', (err, files) => {
+      if (err) reject();
+      //we don't know what the bundle name will be beforehand
+      const cssFile = files.find(file => path.extname(file) === '.css');
+      expect(cssFile).not.toBe(0);
+
+      fs.readFile(`./test/integration/fixtures/autoprefixer-test/test-project/build/static/css/${cssFile}`, 'utf8', (err, data) => {
+        if (err) reject();
+        data = data.substring(0, data.indexOf("/*")-1);
+        expect(data).toBe(expectedStyles);
+        resolve();
+      });
+    });
+  });
+  await prom;
+});
diff --git a/test/integration/fixtures/autoprefixer-test/test-package-files/craco.config.js b/test/integration/fixtures/autoprefixer-test/test-package-files/craco.config.js
@@ -0,0 +1,11 @@
+const webpack = require('webpack');
+const isDevelopment = false;
+module.exports = {
+  style: {
+    postcss: {
+      plugins: [
+        require('autoprefixer')()
+      ]
+    }
+  }
+};
diff --git a/test/integration/fixtures/autoprefixer-test/test-package-files/package.json b/test/integration/fixtures/autoprefixer-test/test-package-files/package.json
@@ -0,0 +1,23 @@
+{
+  "name": "craco-postcss-test",
+  "version": "0.1.0",
+  "private": true,
+  "dependencies": {
+    "react": "^18.2.0",
+    "react-dom": "^18.2.0",
+    "web-vitals": "^3.3.1",
+    "autoprefixer": "^10.4.14"
+  },
+  "scripts": {
+    "start": "craco start",
+    "build": "craco build",
+    "test": "craco test",
+    "eject": "react-scripts eject"
+  },
+  "browserslist": [
+    ">0.2%",
+    "not dead",
+    "not ie <= 11",
+    "not op_mini all"
+  ]
+}
diff --git a/test/integration/fixtures/autoprefixer-test/test-package-files/public/index.html b/test/integration/fixtures/autoprefixer-test/test-package-files/public/index.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8" />
+    <title>React App</title>
+    <link rel="stylesheet" href="index.css" />
+  </head>
+  <body>
+    <div id="root"></div>
+    <script src="index.js"></script>
+  </body>
+</html>
diff --git a/test/integration/fixtures/autoprefixer-test/test-package-files/src/index.css b/test/integration/fixtures/autoprefixer-test/test-package-files/src/index.css
@@ -0,0 +1,12 @@
+::placeholder {
+  color: gray;
+}
+
+.image {
+  background-image: url("https://google.com");
+}
+@media (min-resolution: 2dppx) {
+  .image {
+    background-image: url("http://nxetflix.com");
+  }
+}
diff --git a/test/integration/fixtures/autoprefixer-test/test-package-files/src/index.js b/test/integration/fixtures/autoprefixer-test/test-package-files/src/index.js
@@ -0,0 +1,12 @@
+import React from 'react';
+import ReactDOM from 'react-dom';
+import './index.css';
+
+ReactDOM.render(
+  <React.StrictMode>
+    <div className="container">
+      <h1>Hi!</h1>
+    </div>
+  </React.StrictMode>,
+  document.getElementById('root')
+);
diff --git a/test/integration/fixtures/basic-integration-test/index.test.js b/test/integration/fixtures/basic-integration-test/index.test.js
@@ -4,7 +4,8 @@ const { execSync, spawn } = require('child_process');
 
 beforeAll(async () => {
   // Start a local server to serve the test project
-  const server = spawn('npx', ['serve', '-s', 'build', '-l', global.PORT], {
+  // We cached serve by installing it locally
+  const server = spawn('npx', ['[email protected]', '-s', 'build', '-l', global.PORT], {
     cwd: join(__dirname, 'test-project'),
   });
 
@@ -19,7 +20,7 @@ beforeAll(async () => {
   });
 });
 
-test('Should have the expected styles', async () => {
+test('Should have the expected custom craco variable name', async () => {
   await page.goto(global.URL, { waitUntil: 'domcontentloaded' }); //todo: make the url changeble
 
   const cracoTestText = await page.$eval(

diff --git a/test/integration/fixtures/basic-integration-test/test-package-files/craco.config.js b/test/integration/fixtures/basic-integration-test/test-package-files/craco.config.js
@@ -3,35 +3,6 @@ const isDevelopment = false;
 module.exports = {
   webpack: {
     configure: (webpackConfig) => {
-      if (!isDevelopment) {
-        const reactRefreshPluginIndex = webpackConfig.plugins.findIndex(
-          (plugin) => plugin.constructor.name === 'ReactRefreshPlugin'
-        );
-
-        if (reactRefreshPluginIndex !== -1) {
-          webpackConfig.plugins.splice(reactRefreshPluginIndex, 1);
-        }
-
-        const babelLoader = webpackConfig.module.rules
-          .find(
-            (rule) =>
-              rule.oneOf &&
-              rule.oneOf.find(
-                (r) => r.loader && r.loader.includes('babel-loader')
-              )
-          )
-          .oneOf.find((r) => r.loader && r.loader.includes('babel-loader'));
-
-        const reactRefreshBabelIndex = babelLoader.options.plugins.findIndex(
-          (plugin) =>
-            plugin && plugin.includes && plugin.includes('react-refresh/babel')
-        );
-
-        if (reactRefreshBabelIndex !== -1) {
-          babelLoader.options.plugins.splice(reactRefreshBabelIndex, 1);
-        }
-      }
-
       webpackConfig.plugins.push(
         new webpack.DefinePlugin({
           __CUSTOM_GLOBAL_CONSTANT__: JSON.stringify('CRACO is working!'),