Replace fast-glob with fdir and picomatch, optimize peer dep validation crawler
#952
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 8b4b1a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
askoufis
force-pushed
the
replace-fast-glob
branch
from
738358c
to
a69f627
Compare
askoufis
commented
Apr 19, 2024
| @@ -104,6 +104,7 @@ | |||
| "node-html-parser": "^6.1.1", | |||
| "open": "^7.3.1", | |||
| "path-to-regexp": "^6.2.0", | |||
| "picomatch": "^3.0.1", | |||
There was a problem hiding this comment.
picomatch v4 is the latest, but fdir's peer dependency hasn't been updated to support it. We're not really missing anything by staying on v3 for now.
askoufis
commented
May 6, 2024
| @@ -1,57 +1,83 @@ | |||
| const { posix: path } = require('node:path'); | |||
| const chalk = require('chalk'); | |||
| const glob = require('fast-glob'); | |||
| const { fdir: Fdir } = require('fdir'); | |||
There was a problem hiding this comment.
I rename the import to prevent https://eslint.org/docs/latest/rules/new-cap at the call site.
| pnpmVirtualStoreRelativePath, | ||
| '@seek*/node_modules/@seek/*/package.json', | ||
| ); | ||
| if (isPnpm) { |
There was a problem hiding this comment.
Due to how fdir handles symlinks, I had to special-case some code for pnpm, in addition to the pnpm-related block below.
michaeltaranto
approved these changes
May 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace
fast-globwithfdirandpicomatchfdiris a 0 dependency directory crawler with optional glob support via a peer dependency. For our use cases, we don't need most of the features offast-glob(or dependencies), but we do need globbing, whichfdirsupports viapicomatch, a 0 dependency glob matcher.Replacing globs with simple filters
Due to API differences and some changes in how we crawl in certain cases, I've replaced some globs with simple filters.
Optimize peer dep validation crawler
validatePeerDepswas crawling all ofnode_modules(and.pnpmif present) many times. We now crawl these places just once, and process the results appropriately. This results in a much faster output of the warning messages.Running
sku buildin a moderately sized app where I modifiednode_modulesto induce a peer dependency warning, the warning was output within a few seconds. On sku11.5.0this took >1 minute.Disable peer dependency validation for pnpm
Our current method of peer dep validation doesn't work for pnpm. This is because it relies on crawling through nested
node_modulesdirectories, which pnpm only uses to store binaries.If we really want this functionality for pnpm, we'll need to implement it some other way.
Note
Not marking this change as breaking as it wasn't doing anything in the first place.