bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file
563 stars 71 forks source link

replace rework css with postcss #78

Closed Tidyzq closed 6 years ago

Tidyzq commented 6 years ago

Hi, I just came across the problem mentioned in #28 , and i found a lots of issues with the same problem that caused by rework, so i simply fork this repo and replace rework with postcss(which used by css-loader). However, just one second before i send this PR, i noticed that there is a branch trying to have multiple engines, so i wonder should i send you another PR to write a postcss engine?

In my point of view, it is 4 years since the last update of rework, so maybe we should simply drop it. Also, as css-loader is also using postcss as its parser, it can ensure that the CSS Module syntax won't cause any issues any more.

bholloway commented 6 years ago

Thanks for you pull request.

Definitely been wanting to do postcss but keep the rework engine as an option.

The “multiple engines” branch is the place to be. It has substantial cleanup. My recollection is that it’s working and is good to go but I need to get some automated tests. If you can PR to that I would definitely appreciate that.

In the meantime I’ll try to take a look at your existing work.

stereokai commented 6 years ago

@Tidyzq @bholloway just a quick update: this loader is amazing, it solved many problems for my team since we transitioned to webpack a few months ago. However, we decided to switch to Tidyzq's fork because rework is old and really unnecessary considering the ubiquity of PostCSS. Plus, we need SCSS with CSS Modules today, and a broken resolve-url-loader is unfortunately in the way of the modern syntax. Thanks for understanding.

tpraxl commented 6 years ago

@bholloway did you have a chance to look at this PR? As far as I get it, this would resolve #28. If I get it right, then it would also make this workaround for tailwind css unnecessary (see heading "Laravel Mix"). The tailwind css workaround causes side effects for sass users. But maybe I'm not getting the real problem. Is there anything I can do to help?

bholloway commented 6 years ago

@stereokai Thanks for the kind words. I appreciate your position and yes there has been some neglect here. In development have favoured not breaking existing users and lack of tests has made change hard. I am trying to remedy that but definitely do what you have to for your build.

@tpraxl IMHO this needs to be on an opt-in basis for existing users to manage their change. I have a multiple-engines branch where I hope to integrate this work. This week I am making a concerted effort on tests there. I will then try to integrate the postcss engine. Thanks to @Tidyzq for the PR that kick started this overdue work.

As always I appreciate comments, suggestions, prodding, etc.

stereokai commented 6 years ago

Thank you for your consideration! I wish I could offer help, but as of now I am deep over my head in development anyway. So I'll just wish you good luck and have fun :)

futhr commented 6 years ago

This PR work fine and solve current issues, it just need rebase and happy merge ;)

bholloway commented 6 years ago

I do need to get this in the mainstream release but I will need some more time. I am happy to consider ways of distributing this PR in the interim.

Rather than installing from a github commitish perhaps we could just release this under another dist tag. That would buy me some more time. @futhr would that help?

futhr commented 6 years ago

@bholloway Yes, the PR now lives in a forked master branch so that sounds fine as a temporary solution, thanks :)

bholloway commented 6 years ago

I have published as resolve-url-loader@experiment-postcss. I had to bring in a few more commits but passes the (basic) e2e tests that I have thus far written.

Thanks again to @Tidyzq. Sorry for the delay.

stereokai commented 6 years ago

Noice! Thank you @bholloway! Is there a plan to get this into master some day soon?

bholloway commented 5 years ago

Please refer to #97 as version 3 uses postcss by default.