webpack-contrib / postcss-loader

PostCSS loader for webpack
MIT License
2.86k stars 209 forks source link

v1.2.1 runs fine, but v1.2.2 throws error #163

Closed detj closed 7 years ago

detj commented 7 years ago

This PR (https://github.com/postcss/postcss-loader/pull/161) assumes that a postcss.config.js will always be present. But isn't this file optional? In my project, I was configuring postcss using Webpack 1.0 syntax as mentioned in the Readme.

module.exports = {
  module: {
    loaders: [
      {
        test: /\.css$/,
        loaders: [
          …
          'postcss-loader'
        ]
      }
    ]
  },
  postcss: () => {
    return [
      require('precss'),
      require('autoprefixer')
    ];
  }
}

This works fine in v1.2.1, but breaks in v1.2.2 with the following error

No PostCSS Config found in /path/to/dir

This seems to be a breaking change and not support the Webpack 1.0 configuration model.

ai commented 7 years ago

Nope, in that PR config could be generated from loader options. Check then above.

Tests are passed too.

michael-ciniawsky commented 7 years ago

@detj What is the current version of webpack you use? webpack v1.x ?

detj commented 7 years ago

@ai okay, yes you are right.

@michael-ciniawsky Yeah, I'm using webpack@v1.12.14. Sorry, I should have explained my problem in detail.

So, my project is based on React Redux Starter Kit. It uses webpack@v1.x and has the following postcss config in its webpack config.

webpackConfig.postcss = [
  cssnano({
    autoprefixer : {
      add      : true,
      remove   : true,
      browsers : ['last 2 versions']
    },
    discardComments : {
      removeAll : true
    },
    discardUnused : false,
    mergeIdents   : false,
    reduceIdents  : false,
    safe          : true,
    sourcemap     : true
  })
]

I recently moved my project to a new machine and ran npm install (yeah, I should have used yarn, I know, but otherwise I couldn't have discovered issues like these :)). After installing fresh node_modules, npm run test started failing with the following error.

No PostCSS Config found in: /path/to/dir/where/style/resides

I read in the README of postcss-loader and postcss-load-config that the recommended way to now write postcss config is to create a postcss.config.js file in the root directory or any subdirectory as required.

Creating postcss.config.js and moving the configuration solves my problem, but what I want to understand is why the configuration from webpack not being read?

Also, there now seems to be multiple ways of loading postcss config including adding it in package.json or creating .postcssrc file. Is there any difference among them?

I'm sure I'm missing something, just don't know what exactly :)

Thanks

michael-ciniawsky commented 7 years ago

Creating postcss.config.js and moving the configuration solves my problem, but what I want to understand is why the configuration from webpack not being read?

@detj Yep thats what I'm also interest in, bc normally options in webpack.config.js supersede postcss.config.js

Could you please do the following to help debugging

cd node_modules/postcss-loader
editor index.js

postcss-lodader/index.js

[L47] console.log(params)
[L51] console.log(options)

and run that with options in webpack.config.js + afterwards post the result here please? :)

Also, there now seems to be multiple ways of loading postcss config including adding it in package.json or creating .postcssrc file. Is there any difference among them?

package.json|.postcssrc (JSON) [static] <=> postcss.config.js(JS) [dynamic] :)

package.json

{
   "devDependencies": {
     "cssnano": "^3.10.0",
     "postcss-nested": "^1.0.0"

  },
  "postcss": {
    "parser": "sugarss",
    "plugins": {
      "postcss-nested": {},
      "cssnano": { "autoprefixer": false }
    }
  }
}

postcss.config.js (recommended)

module.exports = (ctx) => ({
   // works for both (file.sss, file.css) automatically
   parser: ctx.webpack.resourcePath.endsWith('.sss') : 'sugarss' : false,
   plugins: {
     'postcss-nested': {}
     'cssnano': ctx.env === 'production' ? {} : false // Will only load in production mode 
  }
})
detj commented 7 years ago

@michael-ciniawsky That makes so much sense! Thank you for the detailed answer :)

Okay, so I added a few console.logs and here's what I discovered.

console.log(params)

{}

console.log(options)

undefined

I also added more logs to debug further

var loader = this;
console.log('loader', loader);
console.log('loader.options', loader.options);
console.log('loader.options.postcss', loader.options.postcss);

// output

loader { data: undefined,
  inputValue: null,
  query: '',
  async: [Function: async],
  callback: [Function] }
loader.options { output:
   { filename: 'extract-text-webpack-plugin-output-filename',
     publicPath: '/_karma_webpack_//' } }
loader.options.postcss undefined

Is this helpful? Let me know if I should do more debugging.

michael-ciniawsky commented 7 years ago

extract-text-webpack-plugin-output-filename

@detj Yep, thx I think we found the 'bug' 😛 , you use extract-text-wbepack-plugin as it seems, which version and could you try if it works without extract-text-webpack-plugin + post your whole webpack.config.js please and say it directly extract-text-webpack-plugin + postcss in it's current state is a troublemaker and straight out mess, we recieve tickets for it nearly every day atm, I already stressed out to fix it in the webpack slack and work on fixing it myself, but the code is very complicated and verbose.

caseyduquettesc commented 7 years ago

@detj In that starter project, I noticed for npm run test it uses the karma.config to build and not the webpack.config. The karma config actually doesn't include any postcss options in it. I think if you add postcss: webpackConfig.postcss right below sassLoader : webpackConfig.sassLoader then the postcss config will be passed into your test bundle and should clear your error.

Although, if you're running tests, you probably don't need to waste the extra build time on css processing, so I would just pass {} instead of webpackConfig.postcss

detj commented 7 years ago

@caseyduquettesc you're right. I did as you said and the tests are passing. But the thing is, tests were passing before and because of some change in this package (my guess), this error started showing up. Anyways, I'll update the react-redux-starter-kit about this development nonetheless. Appreciate it! Thanks.

@michael-ciniawsky okay, will take a look when I have some time. thanks. 👍