fdir icon indicating copy to clipboard operation
fdir copied to clipboard

Feature request: pass picomatch to the builder

Open glenn2223 opened this issue 2 years ago • 3 comments

It would be nice to be able to pass picomatch directly to fdir for better bundling/terser support.

Though I did notice you kind of mention it here, I just wanted to bring it up as a specific use-case feature request.


If I find time and you're open to the idea (to save wasted time 😂), I may be able to submit a PR.

I was thinking that this would have to be a new entry point to make the most sense, though caching becomes an issue. This method would allow for the best portability/adaptability and lean (sort of) towards the pluggability aspect. It would be something like:

globWithMatcher(matcher: Matcher) {
  this.options.filters.push((path) => matcher(path));
  return this;
}

interface Matcher {
  (test: string): boolean;
}

// call it like
new fdir({
    includeBasePath: true,
    resolvePaths: true,
    suppressErrors: true,
    // any other options here
})
    .globWithMatcher(
        picomatch(
            [
                "**/*.scss",
                // Any others
            ],
            {
                ignore: [ 
                    // Stuff
                ],
                dot: true,
                nocase: true,
            }
        )
    )
    .crawl(crawlPath)
    .withPromise();
    

If you'd rather have caching then you could extend the globWithoptions (though it's quite counterintuitive) or change the above method:

type globMatcher<T> = (
  glob: string | string[], 
  options: T
) => Matcher;

interface Matcher {
  (test: string): boolean;
}

// THEN globWithOptions becomes
globWithOptions(
  patterns: string[], 
  options: PicomatchOptions,
  matcher?: globMatcher // removing typing if this is the option
): this;

// OR globWithMatcher becomes
globWithMatcher<T>(
  patterns: string[], 
  options: T,
  matcher: globMatcher
): this;

glenn2223 avatar Feb 14 '23 14:02 glenn2223

@glenn2223 I am not sure I see the benefit of exposing such an API. What's the difference between this and globWithOptions API?

thecodrr avatar Feb 15 '23 05:02 thecodrr

There are a few reasons to have this new API:

  • broaden the glob compatibility
    • moving away from being strictly picomatch (see examples)
    • adding support for micromatch, minimatch, etc
    • users can even create custom functions (by simply implementing a (string) => boolean method)
  • when tersering/bundling, rather than having a reference that you need to successfully require("picomatch) on; you can just pass it directly in a call (this is where the difference/issue with globWithOptions comes in)

Examples (based on name globUsing - not passing any options, for brevity)

new fdir()

// picomatch
  .globUsing(picomatch('*.foo'))

// minimatch
  .globUsing(new minimatch('*.foo').match)

// micromatch
  .globUsing(micromatch.matcher('*.foo'))

// nanomatch
  .globUsing(nanomatch.matcher('*.foo'))

// CUSTOM (just `endsWith`)
  .globUsing(
    (input: string) => 
      input.endsWith('.foo') ||
      input.endsWith('.bar')
  )

// CUSTOM (function where other stuff can happen)
  .globUsing(cM.matcher);
/* WHERE */ const cM = customMatcher('*.foo');

function customMatcher(pattern: string)
{
    // Fake picomatch
    const pM = ((pat) => (input: string) => input.endsWith(pat))(pattern),
    checkedList: string[] = [],
    matchList: string[] = [];

   return { matcher, checkedList, matchList };

   function matcher(input: string) {
        checkedList.push(input);

        const result = pM(input);

        if (result) {
            matchList.push(input);
        }

        return result;
    }
}

// All this is based on:
export declare class Builder<TReturnType extends Output = PathsOutput> {
    globUsing(matcher: Matcher): this;
}

export interface Matcher {
  (test: string): boolean;
}

Now, I know these can be implemented using the filter call. But, it's not really clear that this can be a glob approach (personally speaking)

glenn2223 avatar Feb 15 '23 12:02 glenn2223

I'm landing at this issue due to bundling issues related to the dynamic requiring of picomatch. Is there a chance @glenn2223 's proposal could be considered? Otherwise, I'm not quite sure the best way to ensure the dependency is available in the bundle through a require call without doing some pretty unfortunate stuff with my bundling tools.

louisscruz avatar Nov 07 '23 00:11 louisscruz