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

Proposal: Default removeCR=true on Window OS #195

Closed bholloway closed 3 years ago

bholloway commented 3 years ago

Per #194.

The issue with orphan CR from libsass is not something likely to be fixed soon.

The proposal is to default the removeCR option to true when on the Windows platform (or more specifically any platform where os.EOL includes \r).

If this change were part of the V4 beta then it might drive early adoption of V4.

Please vote with 👍 or 👎 emoji

lucaslisboa commented 3 years ago

I built a script and added it to package.json to run temporarily...

powershell -Command "(gc ./node_modules/resolve-url-loader/index.js) -replace 'removeCR : false', 'removeCR : true' | Out-File -encoding ASCII ./node_modules/resolve-url-loader/index.js"

bholloway commented 3 years ago

Having thought more, I'm going to implement the proposed default.

removeCR : os.EOL.includes('\r'),

Anyone have objections state them now.

bholloway commented 3 years ago

The interesting thing is that libsass claim this bug is fixed.

@shaurcasm @lucaslisboa can you please tell me which version of libsass are you using?

bholloway commented 3 years ago

Published resolve-url-loader@4.0.0-beta.2 as tag next. 🎉

Closing as implemented. Discussion can continue as needed.