fix: resolve conflict #558
Conversation
Codecov Report
@@ Coverage Diff @@
## master #558 +/- ##
==========================================
+ Coverage 97.58% 97.61% +0.03%
==========================================
Files 6 6
Lines 124 126 +2
==========================================
+ Hits 121 123 +2
Misses 3 3
Continue to review full report at Codecov.
|
|
@jhnns can we review and add this fix, please? I had errors with 'v7.0.0' and 'v7.0.1' and with this PR my build is working just fine. |
There was a problem hiding this comment.
Thanks for prodiving test cases to reproduce the problem 👍
I don't think that the fix makes sense here (see my comment). It needs to be addressed in the importsToResolve function
| @@ -46,6 +46,9 @@ function webpackImporter(resourcePath, resolve, addNormalizedDependency) { | |||
| Promise.reject() : | |||
| resolve(dir, importsToResolve[0]) | |||
| .then(resolvedFile => { | |||
| if (!([".sass", ".scss", ".css"].includes(path.extname(resolvedFile)))) { | |||
| throw new Error("Alias should have 'sass', 'scss' or 'css' extension"); | |||
There was a problem hiding this comment.
This requirement is very arbitrary and I don't think that we should introduce that. Why do aliases have to have a sass, scss or css extension? Furthermore, it's not necessarily an alias that caused this condition, so the error message is also misleading.
There was a problem hiding this comment.
@jhnns i know 😞 , it is very interesting. Found solution:
- Add url + all paths from
includePathsin https://github.com/webpack-contrib/sass-loader/blob/master/lib/importsToResolve.js#L56 (i.e.includePath[0]/${url},includePath[1]/${url},url) - Node-sass should be try import first when we try import throw own implement (not sure it is possible)
Any ideas how we can fix this?
There was a problem hiding this comment.
Currently not. I wish we could let node-sass handle the imports as default and then use webpack's resolver, but that's not possible with the current node-sass API.
There was a problem hiding this comment.
@jhnns 😞 I try to search solution todat, have some ideas.
|
This change introduces other problems as described here. I think we need to go a similar route as with the less-loader here. If the Actually, I'm even thinking about making webpack's resolver opt-in because I assume that the threadpool issue with custom importers #147 is the cause for the performance problems #296. Furthermore, most people don't need webpack's resolver. They just want to use Sass the regular way. If the webpack resolver is used, the |
|
@jhnns what about don't resolve path includes |
|
The problem is that we can only know after resolving that an import was supposed to be resolved by |
|
@jhnns |
|
@jhnns i faced with problem in some projects i need I think solution with using only |
No description provided.