search

paulmillr / chokidar

Minimal and efficient cross-platform file watching library
https://paulmillr.com
MIT License
10.8k stars 574 forks source link

feat: move from braces to brace-expansion #1302

Closed 43081j closed 2 months ago

43081j commented 7 months ago

This replaces braces with brace-expansion, a much leaner and more actively maintained package.

paulmillr commented 7 months ago

What's there to maintain? It's probably much slower.

43081j commented 7 months ago

happy to find out if you like! i'll have a look this evening if i can

the leanness is more the reason to move. i do agree there's not much to maintain once this works, other than any security updates, etc.

paulmillr commented 7 months ago

Leanness is:

  1. Rewrite chokidar to drop globbing, decreasing amount of deps to a few.
  2. Switch to typescript.

Started in #1195, but never finished.

43081j commented 7 months ago

do you want any help?

braces is small fish but still responsible for dependency bloat either way. if we can solve that by dropping globbing altogether though, i'm happy to go in whatever direction it is you want

you were right though, brace-expansion in particular is slightly slower than braces. so i'll go open a PR in brace-expansion to fix that whether we use it here or not 👍

paulmillr commented 7 months ago

Yes, if anyone can help on the rewrite, it would be highly appreciated. Non-trivial task.

43081j commented 7 months ago

sure no worries, i'll take a look at it. non trivial esm/ts conversions are my kind of thing 👀

43081j commented 7 months ago

for anyone curious, i've opened a perf fix in juliangruber/brace-expansion#64 which makes it out-perform braces by a fair amount

hellobontempo commented 2 months ago

Hi there! I came across this PR while looking to see if there was chatter about upgrading braces to 3.0.3 because of this security vulnerability: https://security.snyk.io/vuln/SNYK-JS-BRACES-6838727

Happy to file a separate issue if that's preferred!

paulmillr commented 2 months ago

@hellobontempo learn how version ranges work

hellobontempo commented 2 months ago

@paulmillr - I missed the tilda when I skimmed the package. No need to be rude. Have a great day!

paulmillr commented 2 months ago

@hellobontempo you can tell this to other 10 people who've opened same pull requests https://github.com/paulmillr/chokidar/pulls?q=is%3Apr+is%3Aclosed

j-crowe commented 2 months ago

@paulmillr learn how to educate people respectfully. Kindness and respect are fundamental skills. Do better.

43081j commented 2 months ago

@j-crowe i don't think this is constructive. paul's messages aren't so helpful either, i understand, but hes been dealing with the burden of a lot of these issues lately so it is understandable

if you can solve the problem with version ranges, please do share what you used in the end so others can do the same

meanwhile, v4 is being worked on which will drop the dependency

paulmillr commented 2 months ago

@j-crowe kindness, respect and documentation updates wouldn't help me to not get another 10 pull requests replacing ~3.0.2 with ~3.0.3. Everyone would keep doing this because people are convinced automatic NPM dependency scanners are good. They are not: they are mostly evil.

j-crowe commented 2 months ago

@43081j nothing that was said warranted the response from @paulmillr. These responses, especially from an author, taint projects and communities supporting them.

@paulmillr I understand the frustration, but you can still educate people respectfully. Hostile comments only reflect poorly on your own character and the projects you're dedicating yourself to.

Good luck. I appreciate your contributions.

paulmillr commented 2 months ago

@j-crowe there is no support from community. No one wants to touch it. Sometimes contributors like @43081j come. That's it. It's a lot of unpaid work just to get another dozen of issues from users who don't know what they're doing.