thecodrr / fdir

⚡ The fastest directory crawler & globbing library for NodeJS. Crawls 1m files in < 1s
https://thecodrr.github.io/fdir/
MIT License
1.46k stars 51 forks source link

Add (failing) test for filename glob #85

Closed IanVS closed 1 year ago

IanVS commented 1 year ago

Maybe I'm using this wrong, but I would expect the test in this PR to pass. And in fact I have this kind of situation in my storybook PR that is failing as well, which is why I figured I'd put this together and find out whether it's a bug.

I also added jest to devDependencies, since I wasn't able to run the tests without it.

thecodrr commented 1 year ago

I tested around and this can easily be fixed by setting basename property to true in the picomatch matcher function. This effectively makes the glob matcher to match against base names in addition to full paths. This also allows picomatch to match against non-full paths (relative paths, filenames etc.). I think we can make it the default until #81 is merged which adds a new method that exposes the internal picomatch options.

IanVS commented 1 year ago

Thanks for taking a look. I suppose for now, I can just add the crawl path to the start of my globs, right?

Edit, nope, that works in some cases, but not all. Maybe not with symlinks, since it seems to be failing for me with a glob like /Users/ianvs/code/storybook/storybook/sandbox/react-vite-default-js/template-stories/**/*.stories.@(js|jsx|ts|tsx|mdx) (see https://github.com/thecodrr/fdir/issues/23#issuecomment-1281751422)

thecodrr commented 1 year ago

@IanVS

After v6.0.2, this test now passes as follows:

  const api = new fdir()
    .withBasePath()
    .globWithOptions(["a.js"], { basename: true })
    .crawl("__tests__/fixtures");
  const files = await api.withPromise();
  expect(files).toEqual(["__tests__/fixtures/a.js"]);

or as follows:

  const api = new fdir()
    .withBasePath()
    .glob("**/a.js")
    .crawl("__tests__/fixtures");
  const files = await api.withPromise();
  expect(files).toEqual(["__tests__/fixtures/a.js"]);
IanVS commented 1 year ago

Awesome, I'll close this out then, thanks!