Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tinyglobby for globbing #622

Closed
wants to merge 1 commit into from
Closed

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented Aug 20, 2024

the snapshot was updated because tinyglobby returns the files in a different order than glob

https://npmgraph.js.org/?q=glob - 26 dependencies
https://npmgraph.js.org/?q=tinyglobby - 2 dependencies

@bcherny
Copy link
Owner

bcherny commented Aug 20, 2024

I appreciate the PR, but we won't be going forward with it. glob has 140M downloads/week on NPM, vs. tinyglobby's 140k. Let's use the boring option unless there is a strong reason not to.

@bcherny bcherny closed this Aug 20, 2024
@benmccann benmccann deleted the tinyglobby2 branch August 20, 2024 21:34
@benmccann
Copy link
Contributor Author

Thanks for taking a look. Another potential way of reducing the dependencies in this package might be to split it into two packages: a CLI and core library. Most of the dependencies come from the CLI and aren't necessary for folks consuming this package as a library. Would that approach be any more amenable?

@bcherny
Copy link
Owner

bcherny commented Aug 20, 2024

What problem are you trying to solve? Is there a specific issue you're running into? The context could be helpful.

@benmccann
Copy link
Contributor Author

I'm trying to reduce the number of dependencies that need to be installed when installing payload CMS. Almost half of the CMS's dependencies come from this library, but a majority of those really aren't necessary for the core functionality provided by this library

@bcherny
Copy link
Owner

bcherny commented Aug 21, 2024

Gotcha. I just published v15.0.1, which removes 41 dependencies (39% of JSTT's dependencies). Hopefully that helps.

Moving the CLI to a new package would be a reasonable as a way to reduce the size of JSTT's dependency tree, but it will have a cost for consumers -- they will now need to download a separate package, fiddle with keeping the two up to date and in sync, and so on.

If you want to reduce the size of the dependency tree even more, you're more than welcome to open an issue. If there is significant interest from others, I'd be open to making the change.

I know this isn't the outcome you were hoping for, but I hope it's a reasonable compromise. You're also welcome to put up PRs for any low-hanging fruit I missed, along the lines of c09dbcd, a1d22d8, and 108f144.

@benmccann
Copy link
Contributor Author

That is great. Thank you! I really appreciate your efforts on this!

If you want to reduce the size of the dependency tree even more, you're more than welcome to open an issue. If there is significant interest from others, I'd be open to making the change.

Done! I've also proposed another alternative: #623

c09dbcd

Thank you! I had some thoughts on removing cli-color as well. Another way to do it would be to replace it with picocolors, which is quite popular (55m downloads / week). There's a slight blocker, which is that this library uses bright colors and those were only recently added to picocolors and not yet released, but we could consider it in another week or two perhaps

Moving the CLI to a new package would be a reasonable as a way to reduce the size of JSTT's dependency tree, but it will have a cost for consumers -- they will now need to download a separate package, fiddle with keeping the two up to date and in sync, and so on.

I don't think it necessarily would, but we can take the discussion to the issue where I've provided some more details: #623

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants