Feedback

[sindresorhus]

I could use some feedback on the API, docs, and code.

Are the docs clear enough? API nice enough? Anything missing? Any suggestions for better method name than matcher.isMatch()?

Currently the matching is case-insensitive? Is this a good default? Should there be an option to toggle this?

[SamVerschueren]

Are the docs clear enough?

Clean and good examples. I like it :).

API nice enough? Anything missing?

What about making it possible to pass in a string as second argument of matcher.

matcher(['foo', 'bar', 'moo'], '!*oo');

Any suggestions for better method name than matcher.isMatch()?

You could use the one that JavaScript uses (test). But then again, test doesn't say that match about what it returns.

Currently the matching is case-insensitive? Is this a good default? Should there be an option to toggle this?

I think it's a good default but I also think there should be an option to change this.

[sindresorhus]

Thanks for the feedback Sam :)

What about making it possible to pass in a string as second argument of matcher.

I've gone back and forth on that. On one hand, it's a nice flexibility, on another, it's also nice with consistency and the clarity of what it accepts. When someone reads matcher(['foo', 'bar', 'moo'], '!*oo');, it's not immediately clear it supports multiple patterns. When only accepting an array, it is.

You could use the one that JavaScript uses (test). But then again, test doesn't say that match about what it returns.

Hah, yeah I considered .test too, but didn't go with it for the same reason. Also considered .is() and .matches(), but they're not as clear is isMatch(), I think.

I think it's a good default but I also think there should be an option to change this.

Do you have an actual need for that? I usually prefer waiting on adding options until there are real-world needs.

[SamVerschueren]

it's not immediately clear it supports multiple patterns.

RTFM :). But we all know how that goes for users ;).

Also considered .is() and .matches(), but they're not as clear is isMatch(), I think.

isMatch makes it clear it returns a boolean. is looks like string comparison :).

Do you have an actual need for that? I usually prefer waiting on adding option until there are real-world needs.

Not at the moment. Maybe just wait for that until someone needs it and creates an issue.

[sindresorhus]

RTFM :). But we all know how that goes for users ;).

Sure, that will always be needed. I'm just considering whether the slight convenience is worth the resulting slightly less readable code.

isMatch makes it clear it returns a boolean. is looks like string comparison :).

I got .is() from Object.is(), but yeah, .isMatch() is better.

[SamVerschueren]

I used the module a minute a go and maybe got a little confused because of this

matcher(['foo', 'bar', 'moo'], ['!*oo']);  // (input, pattern)
matcher.isMatch('uni*', 'unicorn');        // (pattern, input)

While the docs state

matcher(inputs, patterns)
matcher.isMatch(input, pattern)

The real implementation of isMatch is using (pattern, input) instead of (input, pattern). It should follow the the matcher argument and use (input, pattern).

[sindresorhus]

Oops, I forgot to update the usage examples. Implementation and API docs are correct. Fixed.

[Qix-]

^ That is definitely confusing.

Does this module accept /regexes/?

[sindresorhus]

Does this module accept /regexes/?

No, why would it? The whole point of this module is not having to use regex for simple matching.

[Qix-]

To be able to filter out entries in an array given a regex. That'd be useful.

[sindresorhus]

['foo', 'bar', 'moo'].filter(x => /.*oo/.test(x));

[Qix-]

That works I suppose.

[sindresorhus]

Yeah, I prefer to keep this module focused on doing one thing well. Especially since matching against a regexes I so simple to do in vanilla JS.

[jamestalmage]

A completely different way to go with the API:

import matcher from 'matcher';

const startsWithFoo = matcher('foo*');

['foo', 'late foo', 'bar'].filter(startsWithFoo);

if (startsWIthFoo(str)) {
  // ...
}

[sindresorhus]

@jamestalmage Yeah, I considered a matcher.create(pattern) method. Do you think that would work? Can't replace the current thing though, as it has special handling for negation and multiple patterns.

I initially had the order of matcher.isMatch reversed so you could do matcher.isMatch.bind('foo*'), but ended up reversing the order as it's most common that the main input is the first argument (minimatch and glob does this).

Do you think it makes sense that the main method is the filtering logic? Or should it have been matcher('*foo') and matcher.filter(['foo', 'bar'], ['f*', 'b*'])? I went back and forth on that too...

[jamestalmage]

Why can't negation and arrays of patterns be used in the factory method as well?

const m1 = matcher(['foo*', '!foobar*']);

If you can make the factory method work, and it's performant - I'd like it to be the one and only option. No sense in a filter method at that point, since you can just use.the native array filter.

[dsblv]

Very nice, I once was just about to create something like this.

Any suggestions for better method name than matcher.isMatch()?

I'd personally prefer matcher.test(): it's the name everyone is used to. I automatically read isMatch as "Is this an instance of some class named Match?".

[sindresorhus]

Why can't negation and arrays of patterns be used in the factory method as well?

The filtering is dependent on the input:

matcher/index.js

Lines 48 to 50 in 6337958

	// if first pattern is negated we include
	// everything to match user expectation
	var matches = firstNegated;

It's also a lot faster than using [].filter().

[stevemao]

When someone reads matcher(['foo', 'bar', 'moo'], '!*oo');, it's not immediately clear it supports multiple patterns. When only accepting an array, it is.

What about multimatch? It uses arrify which is the opposite of this module.

[sindresorhus]

@stevemao I made multimatch a long time ago. Would not have done it that way today.