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

Drop fast-glob dependency from svelte-check #2397

Closed
benmccann opened this issue Jun 12, 2024 · 2 comments · Fixed by #2433
Closed

Drop fast-glob dependency from svelte-check #2397

benmccann opened this issue Jun 12, 2024 · 2 comments · Fixed by #2433

Comments

@benmccann
Copy link
Member

benmccann commented Jun 12, 2024

Description

Removing it would remove 18 dependencies: https://npmgraph.js.org/?q=fast-glob

It would still be pulled in via chokidar in the short-term, but chokidar 4 will remove it.

svelte-check pulling in fast-glob is responsible for 16% of the dependencies in a new SvelteKit project

Proposed solution

The usage looks pretty simple and I don't think we need a library to help

Alternatives

Use tiny-glob, which SvelteKit already uses. That won't add dependencies since the user is already downloading tiny-glob. Or maybe switch to fdir

Additional Information, eg. Screenshots

No response

@jasonlyu123
Copy link
Member

It actually should be a devDependencies because it's bundled by rollup. But It can also be removed as part of #2364 since fast-glob doesn't handle it.

@jasonlyu123
Copy link
Member

Found one problem. Although we didn't mention the --ignore option can be a glob in the doc. But because it is directly passed to fast-glob, it can be.

ignore: ['node_modules/**'].concat(filePathsToIgnore.map((ignore) => `${ignore}/**`))

Not sure if we consider this as a breaking change. tiny-glob also doesn't have options for ignore so we can't replace it with it. Or we can replace it with typescript's ts.sys.readDirectory. it only supports limited glob patterns but I doubt anyone would use complex glob patterns here.

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 a pull request may close this issue.

2 participants