feat: add "fileResolve" option for deep import resolving #142
Conversation
|
@KingSora thanks for the PR, I really appreciate your contribution. I have no time to implement features myself now, so this is a huge help for me. I’ll review the PR in a few days. |
|
@madyankin looking forward for the review! I've also created another PR which should fix sourcemaps #143 |
|
@madyankin just a little reminder :) |
|
@KingSora thanks for pinging me. I have a todo in my Things, but now I'm relocating to another country, so a bit overwhelmed. Will have some time for the review next week |
| path.resolve(this.root, relativeDir), | ||
| newPath | ||
| ); | ||
| (useFileResolve |
There was a problem hiding this comment.
Let's rewrite this using async/await. I'm thinking of rewriting all the promises into async/await in the future
| @@ -50,6 +50,11 @@ declare interface Options { | |||
| Loader?: typeof Loader; | |||
|
|
|||
| resolve?: (file: string) => string | Promise<string>; | |||
|
|
|||
| fileResolve?: ( | |||
| @@ -85,6 +85,15 @@ module.exports = (opts = {}) => { | |||
| if (resultPluginIndex === -1) { | |||
| throw new Error("Plugin missing from options."); | |||
| } | |||
| // resolve and fileResolve can't be used together | |||
There was a problem hiding this comment.
Thank you for thinking about backwards compatibility. Anyway, I'd prefer to introduce a breaking change and bump the major version. So, let's remove the original resolve version, replace it with yours, and update the readme. Feel free to use the resolve option name for this.
|
@madyankin please leave it open. I'll come back to it as soon as possible (next week possibly) I just need to finish an different project first |
|
@madyankin on it |
Good day!
I wanted to use the
resolveoption in order to resolve files from thecompose:import. Unfortunately this option only gives me the possibility to resolve the "first layer" of composes.Consider the following setup:
In this setup the
a.cssfile is my entry point. Theresolvefunction will only be fired for thecomposes: b from "./b.css"statement. The "deeper" import insideb.cssis completely opaque.For this usecase I've implemented the
fileResolveoption in a non-breaking manner. This function will be called for allcomposesstatements no matter how deep they are.fileandimporter, both of which are strings. Thefileis essentially the part between the brackets after thefromkeyword (same as the first argument of theresolveoption). Theimporteris the file from which thiscomposestatements comes from.string | nullor aPromise<string | null>. The returnedstringhas to be an absolute path. Ifnullis returned the default resultion takes place.I haven't done that, but with this change I would propose to
deprecatethe currentresolveoption, because the newfileResolveoption is a bit more powerful, still straight forward to use and can still do the same thing as the
resolvefunction.The
READMEwould also need an update, but I would do that after I the feedback from this PR.Fixes: #131