fdir icon indicating copy to clipboard operation
fdir copied to clipboard

feat: allow custom glob functions other than picomatch

Open 43081j opened this issue 1 year ago • 6 comments

This allows you to pass in your own globFunction rather than picomatch, so we're not forcing a choice here (someone may want to use a simpler library or a newer picomatch).

Example:

function globFunction(
  patterns: string | string[],
) {
  return (test) => {
    // some glob match logic
  };
}

const api = new fdir({globFunction})
  .glob('*.js')
  .crawl(cwd);
const files = await api.withPromise();

benchmark seems to be about the same as before on my machine

this also means the options stay strongly typed for whatever glob function you use i think. some example of that:

interface GlobOptions {
  x: number;
}

declare function globFunction(patterns: string|string[], options?: GlobOptions): GlobMatcher;

const api = new fdir({globFunction});

api.globWithOptions(
  ['*.js'],
  {
    x: 100, // works
    y: 200, // errors, since `GlobOptions` has no such prop
  }
);

43081j avatar Jun 24 '24 13:06 43081j

happy to update so we have a setter too if that works!

we can have withGlobFunction or some such thing which sets the same property

will wait and see what @thecodrr thinks 👍

43081j avatar Jun 29 '24 21:06 43081j

should be sorted now 👍

i loosened the types on the way in to unknown and infer the strong types when you call globWithOptions

reason for this is because picomatch exports a conditional type which confuses inference here (we end up with it having a return type of never because the compiler can't infer it)

43081j avatar Jul 02 '24 12:07 43081j

@thecodrr any chance you can take another look at this?

43081j avatar Jul 22 '24 10:07 43081j

@43081j super busy right now but will take a look tomorrow!

thecodrr avatar Jul 27 '24 06:07 thecodrr

all good. let me know if you need any help

same with the other roadmap stuff you wanted to get done, happy to help

43081j avatar Jul 27 '24 08:07 43081j

the documentation.md file should be updated with the new options i think? looks like you missed that

SuperchupuDev avatar Aug 25 '24 09:08 SuperchupuDev

I think before this can be merged, we should test the API with some other glob matching libraries and see how compatible & easy this is to use. The glob & globWithOptions functions were designed to be very picomatch specific so I am not sure if the current design and implementation still makes sense if other libraries are used.

thecodrr avatar Aug 27 '24 07:08 thecodrr

That's true, I'll try it out when I get chance

The types do infer the options parameter but maybe we don't need to do that, and can just rely on people binding it in (instead of having globWithOptions)

43081j avatar Aug 27 '24 08:08 43081j

@thecodrr I tried this with zeptomatch:

 const globFunction = (glob: string) => (path: string): boolean => zeptomatch(glob, path);
 const api = new fdir({globFunction})
   .glob("**/*.js")
   .crawl("node_modules");
 const files = await api.withPromise();

the glob function looks funky here just because zeptomatch doesn't return a factory function, but that's all good. it seems to work fine 👍

43081j avatar Aug 29 '24 12:08 43081j

@thecodrr any chance I can get some time from you to move this along? 🙏

43081j avatar Sep 24 '24 09:09 43081j