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

fix(js): switch from fast-glob to tinyglobby #29141

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

benmccann
Copy link
Contributor

Current Behavior

17 dependencies: https://npmgraph.js.org/?q=fast-glob

Expected Behavior

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

Related Issue(s)

es-tooling/ecosystem-cleanup#117

@benmccann benmccann requested a review from a team as a code owner December 2, 2024 03:55
Copy link

vercel bot commented Dec 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Dec 9, 2024 9:44pm

@JamesHenry
Copy link
Collaborator

Thanks @benmccann got some task failures to take a look

@benmccann
Copy link
Contributor Author

Thanks @JamesHenry. When I run pnpm install it's not updating the package-lock.json file. Is there some special setup this repository has around that? It seems the be the cause of the CI errors. I'm not sure if this is just some issue specific to my machine. If you're not familiar with this issue, any chance you could push a commit updating the lockfile for me?

/home/workflows/workspace/packages/js/package.json
61:5 error The version specifier does not contain the installed version of "tinyglobby" package: 0.2.6 @nx/dependency-checks

@JamesHenry
Copy link
Collaborator

@benmccann The issue is that you haven't added tinyglobby to the root package.json

The repo doesn't (yet) use package manager workspaces, so your reference in the @nx/js package.json doesn't influence the installed packages in any way.

You can verify this with pnpm why:

❯ pnpm why tinyglobby                                                                                                                х 1 24s
Legend: production dependency, optional only, dev only

@nx/nx-source /Users/james/Repos/open-source/nrwl/nx-alt

devDependencies:
nuxt 3.13.2
├─┬ @nuxt/devtools 1.4.2
│ └── tinyglobby 0.2.6
└── tinyglobby 0.2.6

Right now the only way tinyglobby gets into the workspace is via a direct and transitive dependency from the nuxt package.

Therefore the fix is to run pnpm add tinyglobby@^0.2.10

Copy link
Collaborator

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @benmccann, sorry I should have remembered this earlier:

Every time we make one of these kinds of PRs we should add a lint entry to prevent regressions:

"no-restricted-imports": [

(It would be easy for a member of the team not familiar with this effort to add fast-glob back in again)

@benmccann benmccann requested a review from a team as a code owner December 9, 2024 21:42
@JamesHenry JamesHenry merged commit e3f8c81 into nrwl:master Dec 10, 2024
6 checks passed
@JamesHenry JamesHenry changed the title chore(js): switch from fast-glob to tinyglobby fix(js): switch from fast-glob to tinyglobby Dec 10, 2024
@JamesHenry
Copy link
Collaborator

FYI I made it a fix pre merge so it gets called out in the changelog

@benmccann benmccann deleted the tinyglobby branch December 10, 2024 16:45
FrozenPandaz pushed a commit that referenced this pull request Dec 10, 2024
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