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
Replace fast-glob
with fdir
and picomatch
, optimize peer dep validation crawler
#952
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'sku': patch | ||
--- | ||
|
||
Disable peer dependency validation for PNPM | ||
|
||
The method we currently use to validate peer dependencies and warn users about duplicate package is not compatible with how PNPM organizes dependencies in `node_modules`. This feature has been disabled for PNPM repos until further notice. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'sku': patch | ||
--- | ||
|
||
Replace `fast-glob` with `fdir` and `picomatch` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'sku': patch | ||
--- | ||
|
||
Improve performance of peer dependency validation | ||
|
||
Peer dependency validation shoould now complete within a few seconds, rather than a few minutes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,57 +1,83 @@ | ||
const { posix: path } = require('node:path'); | ||
const chalk = require('chalk'); | ||
const glob = require('fast-glob'); | ||
const { fdir: Fdir } = require('fdir'); | ||
|
||
const { cwd: skuCwd } = require('../lib/cwd'); | ||
const { cwd } = require('../lib/cwd'); | ||
const toPosixPath = require('../lib/toPosixPath'); | ||
|
||
const { rootDir, isPnpm } = require('../lib/packageManager'); | ||
const debug = require('debug')('sku:compilePackages'); | ||
|
||
/** @type {string[]} */ | ||
let detectedCompilePackages = []; | ||
|
||
try { | ||
const globs = ['node_modules/@seek/*/package.json']; | ||
const cwd = skuCwd(); | ||
// If there's no rootDir, we're either inside `sku init`, or we can't determine the user's | ||
// package manager. In either case, we can't correctly detect compile packages. | ||
if (rootDir) { | ||
try { | ||
let crawler = new Fdir(); | ||
|
||
if (isPnpm && rootDir) { | ||
const pnpmVirtualStorePath = path.join( | ||
toPosixPath(rootDir), | ||
'node_modules/.pnpm', | ||
); | ||
const pnpmVirtualStoreRelativePath = path.relative( | ||
'.', | ||
pnpmVirtualStorePath, | ||
); | ||
const pnpmVirtualStoreGlob = path.join( | ||
pnpmVirtualStoreRelativePath, | ||
'@seek*/node_modules/@seek/*/package.json', | ||
); | ||
if (isPnpm) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to how |
||
// Follow symlinks inside node_modules into the pnpm virtual store | ||
crawler = crawler.withSymlinks().withRelativePaths(); | ||
} else { | ||
crawler = crawler.withBasePath(); | ||
} | ||
|
||
globs.push(pnpmVirtualStoreGlob); | ||
} | ||
const seekDependencyGlob = '**/@seek/*/package.json'; | ||
|
||
let results = crawler | ||
.glob(seekDependencyGlob) | ||
.crawl('./node_modules/@seek') | ||
.sync(); | ||
|
||
if (isPnpm) { | ||
const pnpmVirtualStorePath = path.join( | ||
toPosixPath(rootDir), | ||
'node_modules/.pnpm', | ||
); | ||
|
||
const pnpmVirtualStoreRelativePath = path.relative( | ||
'.', | ||
pnpmVirtualStorePath, | ||
); | ||
|
||
detectedCompilePackages = glob | ||
.sync(globs, { | ||
cwd, | ||
}) | ||
.map((packagePath) => { | ||
const packageJson = require(path.join(cwd, packagePath)); | ||
|
||
return { | ||
isCompilePackage: Boolean(packageJson.skuCompilePackage), | ||
packageName: packageJson.name, | ||
}; | ||
}) | ||
.filter(({ isCompilePackage }) => isCompilePackage) | ||
.map(({ packageName }) => packageName); | ||
} catch (e) { | ||
console.log( | ||
chalk.red`Warning: Failed to detect compile packages. Contact #sku-support.`, | ||
); | ||
console.error(e); | ||
const pnpmVirtualStoreResults = new Fdir() | ||
.withRelativePaths() | ||
.glob(seekDependencyGlob) | ||
.crawl(pnpmVirtualStoreRelativePath) | ||
.sync(); | ||
|
||
results.push(...pnpmVirtualStoreResults); | ||
|
||
// All results will be relative to the virtual store directory, so we need | ||
// to prepend the relative path from the current directory to the virtual store | ||
results = results.map((file) => | ||
path.join(pnpmVirtualStoreRelativePath, file), | ||
); | ||
} | ||
|
||
detectedCompilePackages = results | ||
.map((packagePath) => { | ||
const packageJson = require(path.join(cwd(), packagePath)); | ||
|
||
return { | ||
isCompilePackage: Boolean(packageJson.skuCompilePackage), | ||
packageName: packageJson.name, | ||
}; | ||
}) | ||
.filter(({ isCompilePackage }) => isCompilePackage) | ||
.map(({ packageName }) => packageName); | ||
} catch (e) { | ||
console.log( | ||
chalk.red`Warning: Failed to detect compile packages. Contact #sku-support.`, | ||
); | ||
console.error(e); | ||
} | ||
} | ||
|
||
debug(detectedCompilePackages); | ||
|
||
module.exports = [ | ||
'sku', | ||
'braid-design-system', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,8 +85,8 @@ | |
"eslint-config-seek": "^12.0.1", | ||
"exception-formatter": "^2.1.2", | ||
"express": "^4.16.3", | ||
"fast-glob": "^3.2.5", | ||
"fastest-validator": "^1.9.0", | ||
"fdir": "^6.1.1", | ||
"find-up": "^5.0.0", | ||
"get-port": "^5.0.0", | ||
"hostile": "^1.3.3", | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
"postcss": "^8.4.0", | ||
"postcss-loader": "^8.0.0", | ||
"prettier": "^2.8.8", | ||
|
@@ -132,6 +133,7 @@ | |
"@types/debug": "^4.1.12", | ||
"@types/express": "^4.17.11", | ||
"@types/minimist": "^1.2.5", | ||
"@types/picomatch": "^2.3.3", | ||
"@types/react": "^18.2.3", | ||
"@types/react-dom": "^18.2.3", | ||
"@types/which": "^3.0.0", | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rename the import to prevent https://eslint.org/docs/latest/rules/new-cap at the call site.