Make resolver filename aware #18
Conversation
|
It looks like line 42 also needs to be updated to accept the filename parameter. Even though it's resolved as a module, IIRC JSON would still fulfill that condition. |
|
Right you are. |
|
Updated. |
|
Great, thanks. Any thoughts on why the test didn't cover/catch that scenario? Can you update the tests for that code path if it's not covered? Also, I like this direction, but (as we discussed in person) I'd like to explore fully committing to metadata such that we can make the API change once and hopefully not have to update it again. For example, could the second argument be an object instead of string? (Open to other suggestions as well.) function handler(value, metadata, cb) {
// metadata -> { filename: 'my/file.json' }
} |
|
Test just didn't extend to that case -- the coverage was the same minimal that another test used. Both could be improved. |
|
I wonder if there's another object to pass instead of arbitrary metadata -- a stream or some input that is actually the thing we're asking about. |
| @@ -193,8 +198,10 @@ exports.create = function create(parent) { | |||
| return handler; | |||
| }); | |||
|
|
|||
| tasks.unshift(function init(done) { | |||
| tasks.unshift(tasks[0].length == 2 ? function init(done) { | |||
| done(null, data); | |||
There was a problem hiding this comment.
can you elaborate on reason for checking the length ?
There was a problem hiding this comment.
Backward compatibility: If the function takes 1 parameter (above), call with value, expect a return value (since there's no callback); if it takes two, pass just the value, last parameter is the callback; if it takes more (new interface!), pass it the value, the filename being processed, and the callback.
|
👍 |
|
Don't merge yet -- still pending some design updates! |
|
Where does this PR stand? Still pending? |
|
This PR is done. I'd be happy having this merged. |
Make resolver filename aware
|
will publish a patch. |
This allows a resolver to take the filename of the currently processed file into account.