egoist / rollup-plugin-postcss

Seamless integration between Rollup and PostCSS.
MIT License
674 stars 213 forks source link

Incorrect postcssOpts.to passed to postcss plugins #65

Open osdevisnot opened 6 years ago

osdevisnot commented 6 years ago

While investigating an issue with path transforms, it seems I might have hit a bug in this plugin. I created a example to reproduce the issue with minimal settings here: https://github.com/osdevisnot/rollup-plugin-postcss-demo

In a nutshell, it seems we are passing incorrect to option to postcss plugins which causes issues with path transformation for postcss-copy-assets

Let me know if I should add more information to this bug report, but the README here summarizes issue correctly:

egoist commented 6 years ago

seems we need to pass the path of extracted file as to, mind sending a pull request to fix that? 😄

osdevisnot commented 6 years ago

I tried looking into this, but I'm little confused for extract: true option. I guess I can try something like this:

if (postcssLoaderOptions.extract) {
    if (typeof postcssLoaderOptions.extract === 'string') {
      postcssLoaderOptions.postcss.to = postcssLoaderOptions.extract
    } else {
      postcssLoaderOptions.postcss.to = path.join('dist', 'bundle.css')
    }
  }

but how do I determine default value for to when extract is set to true as opposed to filename.

egoist commented 6 years ago

hmm I guess to fix this we have to set extract to a string since we don't know where the css should be extracted to during transformation.

when extract: true we may simply use this.id as the fallback for to 😅 also call this.warn(msg) to remind user of the downside of using true for extract

egoist commented 6 years ago

ah seems we need to correctly set to even if it's not extracted..

egoist commented 6 years ago

btw I think you may use https://github.com/borodean/postcss-assets instead

osdevisnot commented 6 years ago

I can probably try looking up into postcss-assets. Meanwhile, would you be interested in a PR to fix .to behavior only for extract = "path"?