tinyglobby icon indicating copy to clipboard operation
tinyglobby copied to clipboard

Should `isDynamicPattern` be a replacement for `is-glob`?

Open benmccann opened this issue 1 year ago • 7 comments

I thought it might allow us to get rid of some more dependencies, but it didn't seem to work when I tried swapping them. Not quite sure why the tests are failing, but you can see my attempt here:

https://github.com/benmccann/json-schema-to-typescript/commit/2a79b100c4e711682e2c5125a78246e4c89dd6c4#diff-fa8d4e24d8399e8350f1c8bad05df53e8032ea995835bf911507015e2db61cddR5

benmccann avatar Nov 11 '24 20:11 benmccann

i can tell you better if you can provide a minimal repro

SuperchupuDev avatar Nov 11 '24 23:11 SuperchupuDev

It looks like isGlob returns false for undefined and isDynamicPattern crashes

benmccann avatar Nov 11 '24 23:11 benmccann

interesting. what does fast-glob do?

SuperchupuDev avatar Nov 12 '24 00:11 SuperchupuDev

throw new TypeError('Patterns must be a string (non empty) or an array of strings');

benmccann avatar Nov 12 '24 00:11 benmccann

i could make it return false on undefined, but would that be ideal for users?

SuperchupuDev avatar Nov 12 '24 10:11 SuperchupuDev

I think that's pretty reasonable behavior. If it's not a string then you're correctly returning that it's not a glob

benmccann avatar Nov 12 '24 14:11 benmccann

throw new TypeError('Patterns must be a string (non empty) or an array of strings');

JFYI: We did this because the pattern is always a string. Anything that is not a string is not a pattern by nature.

One of the interesting byways… we had a case where dozens of projects started to build incorrectly (local & CI) after updating the common library because undefined was passed to fast-glob due to an error, and fast-glob happily and silently returned []. It's a great time to debug :)

mrmlnc avatar Jan 04 '25 10:01 mrmlnc