Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions change/change-21b37dad-e52f-40ef-937e-eb775c98f1e9.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "just-scripts",
"type": "minor",
"dependentChangeType": "patch",
"comment": "Replace `run-parallel-limit` with `p-limit`, and use native promises in `cleanTask`, `copyTask`, and `sassTask`",
"email": "198982749+Copilot@users.noreply.github.com"
}
]
}
3 changes: 1 addition & 2 deletions packages/just-scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"glob": "^13.0.6",
"just-task": "workspace:^",
"nano-spawn": "catalog:prod",
"run-parallel-limit": "^1.0.6",
"p-limit": "^7.3.1",
"supports-color": "^8.1.0",
"webpack-merge": "^6.0.1"
},
Expand Down Expand Up @@ -181,7 +181,6 @@
"devDependencies": {
"@microsoft/api-extractor": "catalog:dev",
"@microsoft/just-internal-scripts": "workspace:^",
"@types/run-parallel-limit": "^1.0.0",
"@types/supports-color": "^8.1.1",
"async-done": "^2.0.0",
"autoprefixer": "catalog:dev",
Expand Down
5 changes: 1 addition & 4 deletions packages/just-scripts/src/tasks/__tests__/cleanTask.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ jest.mock('just-task/lib/logger');

// Mock fse.remove to track calls without actual filesystem operations
jest.mock('fs-extra', () => ({
remove: jest.fn((_path, cb: (err: null) => void) => {
cb(null);
return Promise.resolve();
}),
remove: jest.fn(() => Promise.resolve()),
}));
const removeSpy = fse.remove as jest.MockedFunction<typeof fse.remove>;

Expand Down
8 changes: 7 additions & 1 deletion packages/just-scripts/src/tasks/__tests__/copyTask.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it, jest, afterEach } from '@jest/globals';
import { beforeAll, describe, expect, it, jest, afterEach } from '@jest/globals';
import mockfs from 'mock-fs';
import fse from 'fs-extra';
import { Readable } from 'stream';
Expand All @@ -13,6 +13,12 @@ jest.mock('just-task/lib/logger');
const itWindows = process.platform === 'win32' ? it : it.skip;

describe('copyTask', () => {
beforeAll(async () => {
// Load the ESM-only `p-limit` used by copyTask up front, since it can't be loaded from disk
// while mock-fs is active.
await import('p-limit');
});

afterEach(() => {
mockfs.restore();
// a couple tests mock additional functions
Expand Down
14 changes: 5 additions & 9 deletions packages/just-scripts/src/tasks/cleanTask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import fse from 'fs-extra';
import path from 'path';
import type { TaskFunction } from 'just-task';
import { logger } from 'just-task';
import parallelLimit from 'run-parallel-limit';

export interface CleanTaskOptions {
/**
Expand All @@ -24,16 +23,13 @@ export function defaultCleanPaths(): string[] {
export function cleanTask(options?: CleanTaskOptions): TaskFunction {
const { paths = defaultCleanPaths(), limit = 5 } = options || {};

return function clean(done: (err: Error | null) => void) {
return async function clean() {
logger.info(`Removing [${paths.map(p => path.relative(process.cwd(), p)).join(', ')}]`);

const cleanTasks = paths.map(
cleanPath =>
function (cb: (error: Error | null) => void) {
fse.remove(cleanPath, cb);
},
);
// p-limit is ESM and must be async imported from CJS
const pLimit = (await import('p-limit')).default;
const limiter = pLimit(limit);

parallelLimit(cleanTasks, limit, done);
await Promise.all(paths.map(cleanPath => limiter(() => fse.remove(cleanPath))));
};
}
39 changes: 22 additions & 17 deletions packages/just-scripts/src/tasks/copyTask.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { globSync, hasMagic } from 'glob';
import fse from 'fs-extra';
import path from 'path';
import { pipeline } from 'stream';
import { pipeline } from 'stream/promises';
import type { TaskFunction } from 'just-task';
import { logger } from 'just-task';
import parallelLimit from 'run-parallel-limit';

export interface CopyTaskOptions {
/**
Expand Down Expand Up @@ -38,9 +37,8 @@ export interface CopyTaskOptions {
export function copyTask(options: CopyTaskOptions): TaskFunction {
const { paths, dest, limit = 15 } = options;

return function copy(done) {
return async function copy() {
if (!paths?.length) {
done();
return;
}

Expand All @@ -54,13 +52,30 @@ export function copyTask(options: CopyTaskOptions): TaskFunction {

fse.mkdirpSync(dest);

const copyTasks: parallelLimit.Task<void>[] = [];
// Source/destination pairs for all files to copy, collected before starting any copies
const filesToCopy: { src: string; dest: string }[] = [];

for (const copyPath of normalizedPaths) {
helper(copyPath, getBasePath(copyPath));
}

parallelLimit(copyTasks, limit, done);
// p-limit is ESM and must be async imported from CJS
const pLimit = (await import('p-limit')).default;
const limiter = pLimit(limit);

await Promise.all(
filesToCopy.map(({ src, dest: destPath }) =>
limiter(async () => {
fse.mkdirpSync(path.dirname(destPath));

// Use `pipeline` rather than wiring up `pipe`/`end`/`error` manually: it resolves only
// after the destination has been fully flushed and closed (not merely when the source
// finishes reading), and destroys both streams on error so no file descriptors or
// partial destination files are leaked.
await pipeline(fse.createReadStream(src), fse.createWriteStream(destPath));
}),
),
);

function helper(srcGlob: string, basePath: string) {
// Return absolute paths to ensure path.relative(basePath, matchedPath) works
Expand All @@ -86,17 +101,7 @@ export function copyTask(options: CopyTaskOptions): TaskFunction {

const relativePath = path.relative(basePath, matchedPath);

copyTasks.push(cb => {
const destPath = path.join(dest, relativePath);

fse.mkdirpSync(path.dirname(destPath));

// Use `pipeline` rather than wiring up `pipe`/`end`/`error` manually: it invokes the
// callback exactly once, only after the destination has been fully flushed and closed
// (not merely when the source finishes reading), and destroys both streams on error so
// no file descriptors or partial destination files are leaked.
pipeline(fse.createReadStream(matchedPath), fse.createWriteStream(destPath), err => cb(err || null));
});
filesToCopy.push({ src: matchedPath, dest: path.join(dest, relativePath) });
}
}
};
Expand Down
105 changes: 52 additions & 53 deletions packages/just-scripts/src/tasks/sassTask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { globSync } from 'glob';
import { logger, resolveCwd, type TaskFunction } from 'just-task';
import path from 'path';
import type { AcceptedPlugin } from 'postcss';
import parallelLimit from 'run-parallel-limit';
import { pathToFileURL } from 'url';
import { tryRequire } from '../tryRequire';

Expand All @@ -25,7 +24,7 @@ export interface SassTaskOptions {
export function sassTask(options: SassTaskOptions): TaskFunction {
const { createSourceModule, postcssPlugins = [] } = options;

return function sass(done) {
return async function sass() {
const sassModule = tryRequire<typeof import('sass')>('sass') || tryRequire<typeof import('sass')>('node-sass');
const postcss = tryRequire<typeof import('postcss')>('postcss');
const autoprefixer = tryRequire<typeof import('autoprefixer')>('autoprefixer');
Expand All @@ -42,64 +41,64 @@ export function sassTask(options: SassTaskOptions): TaskFunction {
.filter(Boolean)
.join(', ');
logger.warn(`Required dependencies not found (${missing}), so this task has no effect.`);
done();
return;
}

const autoprefixerFn = autoprefixer({ overrideBrowserslist: ['> 1%', 'last 2 versions', 'ie >= 11'] });
const files = globSync('src/**/*.scss', { absolute: true, cwd: process.cwd() });

const tasks: parallelLimit.Task<void>[] = files.map(fileName => cb => {
fileName = path.resolve(fileName);

// The modern `compile()` API is available in `sass` but not in `node-sass`
if (typeof sassModule.compile === 'function') {
try {
const { css } = sassModule.compile(fileName, {
importers: [{ findFileUrl: patchSassFileUrl }],
loadPaths: [path.resolve(process.cwd(), 'node_modules')],
});
processCss(css);
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
cb(new Error(`${path.relative(process.cwd(), fileName)}: ${message}`));
}
} else {
sassModule.render(
{
file: fileName,
importer: patchSassUrl,
includePaths: [path.resolve(process.cwd(), 'node_modules')],
},
(err, result) => {
if (err || !result) {
cb(new Error(`${path.relative(process.cwd(), fileName)}: ${err || 'no result returned'}`));
return;
// p-limit is ESM and must be async imported from CJS
const pLimit = (await import('p-limit')).default;
const limiter = pLimit(5);

await Promise.all(
files.map(file =>
limiter(async () => {
const fileName = path.resolve(file);

let css: string;
// The modern `compile()` API is available in `sass` but not in `node-sass`
if (typeof sassModule.compile === 'function') {
try {
css = sassModule.compile(fileName, {
importers: [{ findFileUrl: patchSassFileUrl }],
loadPaths: [path.resolve(process.cwd(), 'node_modules')],
}).css;
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
throw new Error(`${path.relative(process.cwd(), fileName)}: ${message}`, { cause: err });
}

processCss(result.css.toString());
},
);
}

function processCss(css: string) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- always defined per above
postcss!([
autoprefixerFn,
...(postcssRtl ? [postcssRtl({})] : []),
...postcssPlugins,
...(clean ? [clean()] : []),
])
.process(css, { from: fileName })
.then(res => {
fs.writeFileSync(fileName + '.ts', createSourceModule(fileName, res.css));
cb(null);
})
.catch(e => cb(e instanceof Error ? e : new Error(String(e))));
}
});

parallelLimit(tasks, 5, done);
} else {
// The legacy `render()` API is callback-based, so it must be promisified
css = await new Promise<string>((resolve, reject) => {
sassModule.render(
{
file: fileName,
importer: patchSassUrl,
includePaths: [path.resolve(process.cwd(), 'node_modules')],
},
(err, result) => {
if (err || !result) {
reject(new Error(`${path.relative(process.cwd(), fileName)}: ${err || 'no result returned'}`));
} else {
resolve(result.css.toString());
}
},
);
});
}

const res = await postcss([
autoprefixerFn,
...(postcssRtl ? [postcssRtl({})] : []),
...postcssPlugins,
...(clean ? [clean()] : []),
]).process(css, { from: fileName });

fs.writeFileSync(fileName + '.ts', createSourceModule(fileName, res.css));
}),
),
);
};
}

Expand Down
35 changes: 17 additions & 18 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2217,13 +2217,6 @@ __metadata:
languageName: node
linkType: hard

"@types/run-parallel-limit@npm:^1.0.0":
version: 1.0.3
resolution: "@types/run-parallel-limit@npm:1.0.3"
checksum: 10c0/499bf6640cf0c9ed6a20922117a247bcdaff8427ebde7f8237333f9966a82affe8504ae9c846193d4d19127be6aa291a10a8221b389a2bc47de6295c49146538
languageName: node
linkType: hard

"@types/send@npm:*":
version: 1.2.1
resolution: "@types/send@npm:1.2.1"
Expand Down Expand Up @@ -6444,7 +6437,6 @@ __metadata:
dependencies:
"@microsoft/api-extractor": "catalog:dev"
"@microsoft/just-internal-scripts": "workspace:^"
"@types/run-parallel-limit": "npm:^1.0.0"
"@types/supports-color": "npm:^8.1.1"
"@types/tar-fs": "npm:^2.0.4"
async-done: "npm:^2.0.0"
Expand All @@ -6461,8 +6453,8 @@ __metadata:
just-task: "workspace:^"
mock-fs: "catalog:dev"
nano-spawn: "catalog:prod"
p-limit: "npm:^7.3.1"
postcss: "catalog:dev"
run-parallel-limit: "npm:^1.0.6"
sass: "catalog:dev"
supports-color: "npm:^8.1.0"
tar-fs: "catalog:dev"
Expand Down Expand Up @@ -7367,6 +7359,15 @@ __metadata:
languageName: node
linkType: hard

"p-limit@npm:^7.3.1":
version: 7.3.1
resolution: "p-limit@npm:7.3.1"
dependencies:
yocto-queue: "npm:^1.2.1"
checksum: 10c0/49b7d7fcc244d4659fa3b260c7b4e632bc72333c60e76e4cd24c7befdeb9b8e22da644e5064e83df9e2ded75245e3e3b9df0e9cf8500b08db8ebf58ac18c6764
languageName: node
linkType: hard

"p-locate@npm:^4.1.0":
version: 4.1.0
resolution: "p-locate@npm:4.1.0"
Expand Down Expand Up @@ -8053,15 +8054,6 @@ __metadata:
languageName: node
linkType: hard

"run-parallel-limit@npm:^1.0.6":
version: 1.1.0
resolution: "run-parallel-limit@npm:1.1.0"
dependencies:
queue-microtask: "npm:^1.2.2"
checksum: 10c0/9c78eb77e788d0ed803a7e80921412f6f6accfb2006de8c21699d9ebf7696df9cefaa313fe14d6169a3fc9f564b34fe91bfd9948cc3a58e2d24136a2390523ae
languageName: node
linkType: hard

"run-parallel@npm:^1.1.9":
version: 1.2.0
resolution: "run-parallel@npm:1.2.0"
Expand Down Expand Up @@ -9753,3 +9745,10 @@ __metadata:
checksum: 10c0/dceb44c28578b31641e13695d200d34ec4ab3966a5729814d5445b194933c096b7ced71494ce53a0e8820685d1d010df8b2422e5bf2cdea7e469d97ffbea306f
languageName: node
linkType: hard

"yocto-queue@npm:^1.2.1":
version: 1.2.2
resolution: "yocto-queue@npm:1.2.2"
checksum: 10c0/36d4793e9cf7060f9da543baf67c55e354f4862c8d3d34de1a1b1d7c382d44171315cc54abf84d8900b8113d742b830108a1434f4898fb244f9b7e8426d4b8f5
languageName: node
linkType: hard