Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "to" postcss option #145

Closed
sormy opened this issue Dec 21, 2018 · 6 comments · Fixed by #163
Closed

Add support for "to" postcss option #145

sormy opened this issue Dec 21, 2018 · 6 comments · Fixed by #163
Labels

Comments

@sormy
Copy link
Contributor

sormy commented Dec 21, 2018

Current version if this plugin has hardcoded "to" value to be the same as "from". This breaks a lot of postcss plugins that are relying on source/target filenames to properly rebase asset urls.

Example of affected plugin: https://github.com/sebastian-software/postcss-smart-asset

Technically, any rebase plugin that is relying on from or to won't work as expected with currently hardcoded from and to.

Would you be open to accept a small PR fix that will do this (so "to" could be explicitly passed when it is needed):

diff -ru rollup-plugin-postcss.old/dist/index.js rollup-plugin-postcss/dist/index.js
--- rollup-plugin-postcss.old/dist/index.js	2019-03-03 16:51:52.000000000 -0500
+++ rollup-plugin-postcss/dist/index.js	2019-03-03 17:01:25.000000000 -0500
@@ -185,7 +185,8 @@
       const postcssOpts = _objectSpread({}, _this.options.postcss, config.options, {
         // Followings are never modified by user config config
         from: _this.id,
-        to: _this.id,
+        // Allow overriding `to` for some plugins that are relying on this value
+        to: options.to || _this.id,
         map: _this.sourceMap ? shouldExtract ? {
           inline: false,
           annotation: false
@@ -669,6 +670,9 @@
     /** Postcss config file */
     config: inferOption(options.config, {}),
 
+    /** Postcss target filename hint, for plugins that are relying on it */
+    to: options.to,
+
     /** PostCSS options */
     postcss: {
       parser: options.parser,
@sormy
Copy link
Contributor Author

sormy commented Dec 28, 2018

Ping!

@sormy
Copy link
Contributor Author

sormy commented Mar 3, 2019

This issue is still actual even for newer version of this plugin. The fix is small, backward compatible and will improve support for other postcss plugins. Please let me know if you are ok with proposed approach and I will created PR with this change. Thank tou.

@sormy
Copy link
Contributor Author

sormy commented Mar 12, 2019

Ping!

@sormy
Copy link
Contributor Author

sormy commented Mar 30, 2019

Here is a PR for this change: #163

@ziofat
Copy link

ziofat commented Apr 6, 2019

postcss-url is also affected by this issue.
I am trying to use background images in css but rollup will not pack them.
I followed #118 to use postcss-url. However, I don't want to use base64 if the image is big. but the option 'rebase' is not working.

@egoist
Copy link
Owner

egoist commented Mar 7, 2020

🎉 This issue has been resolved in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@egoist egoist added the released label Mar 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants