Skip to content

Commit

Permalink
Replace fast-glob with fdir and picomatch
Browse files Browse the repository at this point in the history
  • Loading branch information
askoufis committed Apr 19, 2024
1 parent 6f33953 commit 64a7c0a
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 98 deletions.
7 changes: 7 additions & 0 deletions .changeset/nervous-carpets-call.md
@@ -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 dependes in `node_modules`. This feature has been disabled for PNPM repos until further notice.
5 changes: 5 additions & 0 deletions .changeset/orange-laws-perform.md
@@ -0,0 +1,5 @@
---
'sku': patch
---

Replace `fast-glob` with `fdir` and `picomatch`
5 changes: 5 additions & 0 deletions .changeset/young-seals-shave.md
@@ -0,0 +1,5 @@
---
'sku': patch
---

Improve performance of peer dependency validation file system crawling
3 changes: 1 addition & 2 deletions fixtures/multiple-routes/package.json
Expand Up @@ -11,6 +11,5 @@
"@testing-library/react": "^14.0.0",
"dedent": "^1.5.1",
"sku": "workspace:*"
},
"skuSkipValidatePeerDeps": true
}
}
99 changes: 60 additions & 39 deletions packages/sku/context/defaultCompilePackages.js
@@ -1,55 +1,76 @@
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');

/** @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) {
// Follow symlinks inside node_modules into the pnpm virtual store
crawler = crawler.withSymlinks().withRelativePaths();
} else {
crawler = crawler.withBasePath();
}

globs.push(pnpmVirtualStoreGlob);
}
let results = crawler
.filter((file) => file.endsWith('package.json'))
.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('@seek*/node_modules/@seek/*/package.json')
.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);
}
}

module.exports = [
Expand Down
12 changes: 9 additions & 3 deletions packages/sku/lib/validateLessFiles.js
@@ -1,7 +1,6 @@
const fs = require('node:fs');
const path = require('node:path');
const glob = require('fast-glob');
const chalk = require('chalk');
const { fdir: Fdir } = require('fdir');

const printBanner = require('./banner');
const exists = require('./exists');
Expand All @@ -14,11 +13,18 @@ module.exports = async () => {
const lessFileGlobResults = await Promise.all(
srcPathsExist
.filter((srcPath) => srcPath && fs.statSync(srcPath).isDirectory())
.map(async (srcPath) => await glob(path.join(srcPath, '**/*.less'))),
.map(async (srcPath) =>
new Fdir()
.filter((file) => file.endsWith('.less'))
.crawl(srcPath)
.withPromise(),
),
);

const srcHasLessFiles = lessFileGlobResults.some(
(fileArray) => fileArray.length > 0,
);

if (srcHasLessFiles) {
printBanner('warning', 'LESS styles detected', [
`Support for ${chalk.bold('LESS')} has been deprecated.`,
Expand Down
52 changes: 33 additions & 19 deletions packages/sku/lib/validatePeerDeps.js
@@ -1,13 +1,13 @@
const { getWhyCommand } = require('./packageManager');
const { getWhyCommand, isPnpm } = require('./packageManager');

const { readFile } = require('node:fs/promises');
const glob = require('fast-glob');
const { fdir: Fdir } = require('fdir');
const semver = require('semver');
const chalk = require('chalk');

const banner = require('./banner');
const track = require('../telemetry');
const { cwd, getPathFromCwd } = require('../lib/cwd');
const { getPathFromCwd } = require('../lib/cwd');
const { paths } = require('../context');

/**
Expand All @@ -21,30 +21,44 @@ const asyncMap = (list, fn) => {
const singletonPackages = ['@vanilla-extract/css'];

module.exports = async () => {
if (isPnpm) {
// pnpm doesn't nest dependencies in the same way as yarn or npm, so the method used below won't
// work for detecting duplicate packages
return;
}

try {
/** @type {string[]} */
const packages = [];

for (const packageName of [
...paths.compilePackages,
...singletonPackages,
]) {
const results = await glob(
[
`node_modules/${packageName}/package.json`,
`node_modules/**/node_modules/${packageName}/package.json`,
],
{
cwd: cwd(),
},
const packagesToCheck = [...paths.compilePackages, ...singletonPackages];
const packageNameByPattern = Object.fromEntries(
packagesToCheck.map((packageName) => [
`node_modules/${packageName}/package.json`,
packageName,
]),
);

const patterns = Object.keys(packageNameByPattern);

const results = await new Fdir()
.withBasePath()
.filter((file) => patterns.some((pattern) => file.endsWith(pattern)))
.crawl('node_modules')
.withPromise();

for (const [pattern, packageName] of Object.entries(packageNameByPattern)) {
const resultsForPackage = results.filter((result) =>
result.endsWith(pattern),
);

if (results.length > 1) {
if (resultsForPackage.length > 1) {
const messages = [
chalk`Multiple copies of {bold ${packageName}} are present in node_modules. This is likely to cause errors, but even if it works, it will probably result in an unnecessarily large bundle size.`,
];

messages.push(
results
resultsForPackage
.map((depLocation) => {
const { version } = require(getPathFromCwd(depLocation));

Expand All @@ -64,9 +78,9 @@ module.exports = async () => {
compile_package: packageName,
});
banner('error', 'Error: Duplicate packages detected', messages);
}

packages.push(...results);
packages.push(...resultsForPackage);
}
}

const compilePackages = new Map();
Expand Down
4 changes: 3 additions & 1 deletion packages/sku/package.json
Expand Up @@ -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",
Expand All @@ -104,6 +104,7 @@
"node-html-parser": "^6.1.1",
"open": "^7.3.1",
"path-to-regexp": "^6.2.0",
"picomatch": "^3.0.1",
"postcss": "^8.4.0",
"postcss-loader": "^8.0.0",
"prettier": "^2.8.8",
Expand Down Expand Up @@ -133,6 +134,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",
Expand Down
10 changes: 6 additions & 4 deletions packages/sku/scripts/init.js
Expand Up @@ -21,7 +21,7 @@ const install = require('../lib/install');
const banner = require('../lib/banner');
const toPosixPath = require('../lib/toPosixPath');
const trace = require('debug')('sku:init');
const glob = require('fast-glob');
const { fdir: Fdir } = require('fdir');
const ejs = require('ejs');

const args = require('../config/args');
Expand Down Expand Up @@ -152,9 +152,11 @@ const getTemplateFileDestinationFromRoot =
process.chdir(root);

const templateDirectory = path.join(toPosixPath(__dirname), '../template');
const templateFiles = await glob(`${templateDirectory}/**/*`, {
onlyFiles: true,
});

const templateFiles = await new Fdir()
.withBasePath()
.crawl(templateDirectory)
.withPromise();

const getTemplateFileDestination = getTemplateFileDestinationFromRoot(
root,
Expand Down

0 comments on commit 64a7c0a

Please sign in to comment.