FIX #25 : add support for a custom mount function...

[zipang]

…to build the desired output.

Principle :
If the user provides a custom function with opts.mount,
the absolute or relative path of matched files is given to that function to return whatever the user really needs.

Test has been added to show how to create a pseudo VFile object in one pass.

NB : the test 'glob: options.cwd' has been corrected as it incorrectly expected '../child' as a result for a path relative to himself. The correct response is in fact '' (empty path!) as shown by :

> path.relative('test/fixture/one/child', 'test/fixture/one/child/../child')
''

[terkelg]

I think this was working as expected. Pretty sure that's how other glob implementations works, including bash and other JS packages like node-glob. Instead of returning an empty string it should return the name of the folder

[lukeed]

I believe so too

[terkelg]

Thanks for the PR. I'll check it out.

What do you think @lukeed?

[lukeed]

I'd much rather see this approach, if any is to be taken, which is what this PR boils down to:

// rename "mount" -> "transform"
opts.transform = (opts.transform || opts.absolute && resolve || relative).bind(null, cwd);

return await walk(matches, glob.base, path, opts, '.', 0);

Current PR trades 3 extra function calls per file (always) in the place of one conditional loop, that only executed one 1 function per file. Not actually a good trade 😬

[zipang]

@terkelg @lukeed : Hi, congrats ! And thanks for the quick review ! :)

Well.. Is there a benchmark somewhere with some serious file hierarchy to scan ?
Could we run it with these 3 scenarios : returning relative paths, absolute paths or a custom object ?

Because, in fact, i believe that the whole point of my PR was to state the fact that returning (very fast) an array of relative paths is great, but not so usefull in the end, because usually, people will want to do something with these paths : like loading stats, examining content.. whatever...
So, if tiny-glob was able to load a more complex representation of a file during the build pass, that's where the actual gain would be.
It's my guess.. (and maybe it's not true..)

So, i would be really interested by a benchmark with thousands of files that would compare these 2 approaches :

glob('whatever').map(path => toVfile(path)) // return paths then map to vfiles
vs
glob('whatever', { transform: toVfile }) // build vfile at the creation

NB : i think i could have done better with my PR if the name and usage of the vars in the walk method were more obvious because i think that calling relative(cwd, path) on each file could be avoided. Some inline doc could be useful for external contributors ! :)

[lukeed]

Well, conceptually, running 3 functions instead of 1 function is slower -- there's no disputing that.

I don't disagree with your idea for a custom transformer; I'm just saying that it's not up to me whether or not the feature is added. However, if it is to be, my snippet is how we'd want to go about it. It applies the ability to transform each file with the desired effect using one function call per file instead of three per file.

The benchmark you're interested in is just comparing a for loop to a Array.p.map. Since the array & transforming function (which is where most of the time will be spent) are the same, this is the only difference. I have some microbenchmarks that cover this exact use case with big numbers, albeit slightly outdated now.

[terkelg]

I think it could be cool with the option to do a custom transform function if we can implement it in a fast and simple way. I guess we can have some sort of default transform function you can overwrite with option.transform

[lukeed]

The path.relative would be the default transformer, so as to prevent any API breaks. It gets changed to path.absolute if options.absolute, or can be whatever was passed into options.transform directly.