refactor(support)!: replace glob package with native node:fs glob (#22735)

BREAKING CHANGE: @appium/support's fs.glob option shape changed from the glob package's GlobOptions to a smaller, @appium/support-owned GlobOptions interface. Supported fields: cwd, withFileTypes, absolute, lazy. Options like nodir/ignore/nocase are no longer supported directly — callers needing directory filtering can pass withFileTypes: true and filter on .isFile()/.isDirectory(); callers needing path exclusion can filter the returned array by pattern.
This commit is contained in:
Mykola Mokhnach
2026-09-07 17:38:56 +02:00
committed by GitHub
parent 8840f9f932
commit add1615607
17 changed files with 190 additions and 46 deletions
+7 -1
View File
@@ -9550,6 +9550,7 @@
"version": "13.0.6",
"resolved": "https://registry.npmjs.org/glob/-/glob-13.0.6.tgz",
"integrity": "sha512-Wjlyrolmm8uDpm/ogGyXZXb1Z+Ca2B8NbJwqBVg0axK9GbBeoS7yGV6vjXnYdGm6X53iehEuxxbyiKp8QmN4Vw==",
"dev": true,
"license": "BlueOak-1.0.0",
"dependencies": {
"minimatch": "^10.2.2",
@@ -9578,6 +9579,7 @@
"version": "4.0.4",
"resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz",
"integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==",
"dev": true,
"license": "MIT",
"engines": {
"node": "18 || 20 || >=22"
@@ -9587,6 +9589,7 @@
"version": "5.0.9",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz",
"integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==",
"dev": true,
"license": "MIT",
"dependencies": {
"balanced-match": "^4.0.2"
@@ -9599,6 +9602,7 @@
"version": "11.3.6",
"resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.3.6.tgz",
"integrity": "sha512-Gf/KoL3C/MlI7Bt0PGI9I+TeTC/I6r/csU58N4BSNc4lppLBeKsOdFYkK+dX0ABDUMJNfCHTyPpzwwO21Awd3A==",
"dev": true,
"license": "BlueOak-1.0.0",
"engines": {
"node": "20 || >=22"
@@ -9608,6 +9612,7 @@
"version": "10.2.5",
"resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz",
"integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==",
"dev": true,
"license": "BlueOak-1.0.0",
"dependencies": {
"brace-expansion": "^5.0.5"
@@ -9623,6 +9628,7 @@
"version": "7.1.3",
"resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz",
"integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==",
"dev": true,
"license": "BlueOak-1.0.0",
"engines": {
"node": ">=16 || 14 >=14.17"
@@ -9632,6 +9638,7 @@
"version": "2.0.2",
"resolved": "https://registry.npmjs.org/path-scurry/-/path-scurry-2.0.2.tgz",
"integrity": "sha512-3O/iVVsJAPsOnpwWIeD+d6z/7PmqApyQePUtCndjatj/9I5LylHvt5qluFaBT3I5h3r1ejfR056c+FCv+NnNXg==",
"dev": true,
"license": "BlueOak-1.0.0",
"dependencies": {
"lru-cache": "^11.0.0",
@@ -19642,7 +19649,6 @@
"archiver": "8.0.0",
"asyncbox": "6.4.2",
"axios": "1.19.0",
"glob": "13.0.6",
"klaw": "4.1.0",
"lockfile": "1.0.4",
"mime-types": "3.0.2",
@@ -3,6 +3,7 @@ import path from 'node:path';
import {fs, util} from '@appium/support';
import type {DriverType, ExtensionType, PluginType} from '@appium/types';
import type {ExtManifest, ExtPackageJson, ExtRecord, InternalMetadata, ManifestData} from 'appium/types/index.js';
import {asyncmap} from 'asyncbox';
import * as YAML from 'yaml';
import {CURRENT_SCHEMA_REV, DRIVER_TYPE, PLUGIN_TYPE} from '../../constants.js';
@@ -16,6 +17,8 @@ import {INSTALL_TYPE_DEV, INSTALL_TYPE_NPM} from './install-types.js';
import {migrate} from './migrations.js';
import {manifestValidator} from './validator.js';
const MAX_CONCURRENT_FS_TASKS = 10;
const CONFIG_DATA_DRIVER_KEY = `${DRIVER_TYPE}s` as const;
const CONFIG_DATA_PLUGIN_KEY = `${PLUGIN_TYPE}s` as const;
@@ -228,10 +231,7 @@ export class Manifest {
const queue: Promise<void>[] = [onMatch(path.join(this.#appiumHome, 'package.json'), true)];
const filepaths = await fs.glob('node_modules/{*,@*/*}/package.json', {
cwd: this.#appiumHome,
absolute: true,
});
const filepaths = await findNodeModulesPackageJsons(path.join(this.#appiumHome, 'node_modules'));
for (const filepath of filepaths) {
queue.push(onMatch(filepath));
}
@@ -356,6 +356,52 @@ export class Manifest {
}
}
/**
* Finds `package.json` paths for packages directly under `node_modules` (and one level deeper for
* scoped `@foo/bar` packages). Unlike `fs.glob`, this follows symlinks, which is how `npm install
* <local-path>` (and `npm link`) install packages.
*/
async function findNodeModulesPackageJsons(nodeModulesDir: string): Promise<string[]> {
let entries: string[];
try {
entries = await fs.readdir(nodeModulesDir);
} catch (err) {
if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') {
return [];
}
throw err;
}
const pkgJsonPathsByEntry = await asyncmap(
entries.filter((entry) => !entry.startsWith('.')),
async (entry): Promise<string[]> => {
const entryPath = path.join(nodeModulesDir, entry);
if (!entry.startsWith('@')) {
const pkgJsonPath = path.join(entryPath, 'package.json');
return (await fs.exists(pkgJsonPath)) ? [pkgJsonPath] : [];
}
let scopedEntries: string[];
try {
scopedEntries = await fs.readdir(entryPath);
} catch {
return [];
}
const scopedPkgJsonPaths = await asyncmap(
scopedEntries.filter((scopedEntry) => !scopedEntry.startsWith('.')),
async (scopedEntry) => {
const pkgJsonPath = path.join(entryPath, scopedEntry, 'package.json');
return (await fs.exists(pkgJsonPath)) ? pkgJsonPath : null;
},
{concurrency: MAX_CONCURRENT_FS_TASKS},
);
return scopedPkgJsonPaths.filter((pkgJsonPath) => pkgJsonPath !== null);
},
{concurrency: MAX_CONCURRENT_FS_TASKS},
);
return pkgJsonPathsByEntry.flat();
}
function isExtension(value: unknown): value is ExtPackageJson<ExtensionType> {
return (
util.isPlainObject(value) &&
@@ -80,7 +80,8 @@ describe('AppiumDriver', function () {
MockConfig.updateBuildInfo.resetBehavior();
MockConfig.updateBuildInfo.resolves();
({AppiumDriver} = await import(`../../lib/appium.js?t=${importCounter++}`));
const mod = await import(`../../lib/appium.js?t=${importCounter++}`);
({AppiumDriver} = mod);
});
afterEach(function () {
@@ -67,7 +67,8 @@ describe('DriverConfig', function () {
manifest = Manifest.getInstance('/somewhere/');
resetMockDefaults(mocks);
mocks.MockAppiumSupport.fs.readFile.resolves(yamlFixture);
({DriverConfig} = await import(`../../../lib/extension/driver-config.js?t=${importCounter++}`));
const mod = await import(`../../../lib/extension/driver-config.js?t=${importCounter++}`);
({DriverConfig} = mod);
resetSchema();
});
@@ -130,7 +130,8 @@ describe('ExtensionCommand', function () {
debug: sandbox.stub(),
} as unknown as AppiumLogger;
({injectAppiumSymlinks} = await import(`../../../lib/cli/extension-command.js?t=${importCounter++}`));
const mod = await import(`../../../lib/cli/extension-command.js?t=${importCounter++}`);
({injectAppiumSymlinks} = mod);
});
afterEach(function () {
@@ -40,10 +40,10 @@ describe('ExtensionConfig', function () {
resetMockDefaults(mocks);
MockAppiumSupport = mocks.MockAppiumSupport;
MockNpm = mocks.MockNpm;
({ExtensionConfig, resolveEsmEntryPoint} = await import(
`../../../lib/extension/extension-config.js?t=${importCounter++}`
));
({Manifest} = await import(`../../../lib/extension/manifest/manifest.js?t=${importCounter++}`));
const extensionConfigMod = await import(`../../../lib/extension/extension-config.js?t=${importCounter++}`);
({ExtensionConfig, resolveEsmEntryPoint} = extensionConfigMod);
const manifestMod = await import(`../../../lib/extension/manifest/manifest.js?t=${importCounter++}`);
({Manifest} = manifestMod);
});
afterEach(function () {
@@ -1,5 +1,6 @@
import assert from 'node:assert/strict';
import {promises as fs} from 'node:fs';
import path from 'node:path';
import {describe, it, beforeEach, afterEach, before, after, mock} from 'node:test';
import type {DriverType, PluginType} from '@appium/types';
@@ -42,7 +43,8 @@ describe('Manifest', function () {
resetMockDefaults(mocks);
migrateStub.resolves();
MockAppiumSupport.fs.readFile.resolves(yamlFixture);
({Manifest} = await import(`../../../lib/extension/manifest/manifest.js?t=${importCounter++}`));
const mod = await import(`../../../lib/extension/manifest/manifest.js?t=${importCounter++}`);
({Manifest} = mod);
});
describe('class method', function () {
@@ -526,12 +528,31 @@ describe('Manifest', function () {
describe('when the underlying implementation emits "error"', function () {
beforeEach(function () {
MockAppiumSupport.fs.glob.rejects(new Error('bogus'));
MockAppiumSupport.fs.readdir.rejects(new Error('bogus'));
});
it('should reject', async function () {
await assert.rejects(manifest.syncWithInstalledExtensions(), /bogus/);
});
});
describe('when node_modules contains dot-prefixed entries', function () {
const nodeModulesDir = path.join('/some/path', 'node_modules');
const scopedDir = path.join(nodeModulesDir, '@scope');
beforeEach(function () {
MockAppiumSupport.fs.readdir.withArgs(nodeModulesDir).resolves(['.hidden-driver', 'normal-driver', '@scope']);
MockAppiumSupport.fs.readdir.withArgs(scopedDir).resolves(['.hidden-scoped', 'visible-scoped']);
});
it('should not probe dot-prefixed package directories', async function () {
await manifest.syncWithInstalledExtensions();
const probedPaths = MockAppiumSupport.fs.exists.getCalls().map((call: any) => call.args[0] as string);
assert.ok(!probedPaths.some((p) => p.includes('.hidden-driver')));
assert.ok(!probedPaths.some((p) => p.includes('.hidden-scoped')));
assert.ok(probedPaths.some((p) => p.includes('normal-driver')));
assert.ok(probedPaths.some((p) => p.includes('visible-scoped')));
});
});
});
describe('hasDriver()', function () {
+3 -3
View File
@@ -16,7 +16,7 @@ export interface MockAppiumSupportFs {
readFile: SinonStub;
writeFile: SinonStub;
walk: SinonStub;
glob: SinonStub;
readdir: SinonStub;
mkdirp: SinonStub;
exists: SinonStub;
realpath: SinonStub;
@@ -86,7 +86,7 @@ export function initMocks(sandbox = createSandbox()): InitMocksResult {
walk: sandbox.stub().returns({
[Symbol.asyncIterator]: sandbox.stub().returns({next: sandbox.stub().resolves({done: true})}),
}),
glob: sandbox.stub().resolves([]),
readdir: sandbox.stub().resolves([]),
mkdirp: sandbox.stub().resolves(),
exists: sandbox.stub().resolves(true),
realpath: sandbox.stub().callsFake(async (p: string) => p),
@@ -156,7 +156,7 @@ export function resetMockDefaults(mocks: InitMocksResult): void {
MockAppiumSupport.fs.walk.returns({
[Symbol.asyncIterator]: mocks.sandbox.stub().returns({next: mocks.sandbox.stub().resolves({done: true})}),
});
MockAppiumSupport.fs.glob.resolves([]);
MockAppiumSupport.fs.readdir.resolves([]);
MockAppiumSupport.fs.mkdirp.resolves();
MockAppiumSupport.fs.exists.resolves(true);
MockAppiumSupport.env.resolveAppiumHome.resolves('/some/path');
@@ -44,7 +44,8 @@ describe('package-changed', function () {
beforeEach(async function () {
resetMockDefaults(mocks);
({packageDidChange} = await import(`../../../lib/utils/package-changed.js?t=${importCounter++}`));
const mod = await import(`../../../lib/utils/package-changed.js?t=${importCounter++}`);
({packageDidChange} = mod);
});
describe('packageDidChange()', function () {
@@ -56,7 +56,8 @@ describe('PluginConfig', function () {
manifest = Manifest.getInstance('/somewhere/');
resetMockDefaults(mocks);
mocks.MockAppiumSupport.fs.readFile.resolves(yamlFixture);
({PluginConfig} = await import(`../../../lib/extension/plugin-config.js?t=${importCounter++}`));
const mod = await import(`../../../lib/extension/plugin-config.js?t=${importCounter++}`);
({PluginConfig} = mod);
resetSchema();
});
@@ -30,6 +30,7 @@ describe('schema', function () {
// fresh (cache-busted) each test isolates that singleton per test, same as the explicit
// `resetSchema()` call below does for the parts reachable through its public API.
beforeEach(async function () {
const mod = await import(`../../../lib/schema/schema.js?t=${importCounter++}`);
({
SchemaFinalizationError,
SchemaUnknownSchemaError,
@@ -43,7 +44,7 @@ describe('schema', function () {
getDefaultsForSchema,
flattenSchema,
validate,
} = await import(`../../../lib/schema/schema.js?t=${importCounter++}`));
} = mod);
resetSchema();
});
@@ -527,7 +527,11 @@ function verifyAppExtension(app: string, supportedAppExtensions: string[]): stri
}
async function calculateFolderIntegrity(folderPath: string): Promise<number> {
return (await fs.glob('**/*', {cwd: folderPath})).length;
let count = 0;
for await (const _entry of fs.glob('**/*', {cwd: folderPath, lazy: true})) {
count++;
}
return count;
}
async function calculateFileIntegrity(filePath: string): Promise<string> {
+14 -14
View File
@@ -1,5 +1,5 @@
import {createHash} from 'node:crypto';
import nativeFs from 'node:fs';
import nativeFs, {type Dirent} from 'node:fs';
import path from 'node:path';
import type Stream from 'node:stream';
@@ -7,7 +7,6 @@ import {fs, timing, util} from '@appium/support';
import type {AppiumLogger} from '@appium/types';
import AsyncLock from 'async-lock';
import {asyncmap} from 'asyncbox';
import type {Path} from 'path-scurry';
import type WebSocket from 'ws';
import type {ItemOptions, StorageItem} from './types.js';
@@ -32,17 +31,17 @@ export class Storage {
}
async list(): Promise<StorageItem[]> {
const items = (await this._listFiles()).filter((p) => !p.fullpath().endsWith(TMP_EXT));
const items = (await this._listFiles()).filter((item) => !item.name.endsWith(TMP_EXT));
if (util.isEmpty(items)) {
return [];
}
const stats = await asyncmap(items, (item) => fs.stat(item.fullpath()), {
const stats = await asyncmap(items, (item) => fs.stat(this._toFullPath(item)), {
concurrency: MAX_TASKS,
});
return items.map((item, index) => ({
name: path.basename(item.fullpath()),
path: item.fullpath(),
name: item.name,
path: this._toFullPath(item),
size: stats[index].size,
}));
}
@@ -82,8 +81,8 @@ export class Storage {
}
const files = (await this._listFiles())
.map((p) => p.fullpath())
.filter((fullPath) => !this._shouldPreserveFiles || path.basename(fullPath).toLowerCase().endsWith(TMP_EXT));
.filter((item) => !this._shouldPreserveFiles || item.name.toLowerCase().endsWith(TMP_EXT))
.map((item) => this._toFullPath(item));
if (util.isEmpty(files)) {
return;
}
@@ -126,12 +125,13 @@ export class Storage {
}
}
private async _listFiles(): Promise<Path[]> {
const paths = (await fs.glob('*', {
cwd: this._root,
withFileTypes: true,
})) as unknown as Path[];
return paths.filter((item) => item.isFile());
private async _listFiles(): Promise<Dirent[]> {
const items = await fs.glob('*', {cwd: this._root, withFileTypes: true});
return items.filter((item) => item.isFile());
}
private _toFullPath(item: Dirent): string {
return path.join(item.parentPath, item.name);
}
private async _addFromStream(opts: ItemOptions, source: Stream): Promise<void> {
+62 -7
View File
@@ -5,6 +5,7 @@ import {
type CopyOptions,
createReadStream,
createWriteStream,
type Dirent,
type MakeDirectoryOptions,
open,
type PathLike,
@@ -17,8 +18,6 @@ import {
import path from 'node:path';
import {promisify} from 'node:util';
import {glob} from 'glob';
import type {GlobOptions} from 'glob';
import klaw from 'klaw';
import type {Walker} from 'klaw';
import sanitize from 'sanitize-filename';
@@ -64,6 +63,43 @@ export interface MvOptions {
*/
export type WalkDirCallback = (itemPath: string, isDirectory: boolean) => boolean | void | Promise<boolean | void>;
/** Options for {@linkcode fs.glob}. Deliberately narrower than `node:fs`'s own `GlobOptions` so callers are insulated from its shape. */
export interface GlobOptions {
/**
* Current working directory to resolve `pattern` and results against.
* @default process.cwd()
*/
cwd?: string;
/** Yield `Dirent`s instead of path strings. */
withFileTypes?: boolean;
/** Yield absolute paths instead of paths relative to `cwd`. Ignored when `withFileTypes` is set (a `Dirent`'s location is already recoverable via `parentPath`). */
absolute?: boolean;
/** Return an async generator that yields matches lazily instead of resolving to an array of all of them. */
lazy?: boolean;
}
/**
* Overloaded call signature for {@linkcode fs.glob}, narrowing its return type based on
* `withFileTypes`/`lazy`. When either flag is a non-literal `boolean` (e.g. a `GlobOptions`
* variable), the return type widens to a union instead of picking a single (possibly wrong) shape.
*/
export interface GlobFn {
(
pattern: string | readonly string[],
options: GlobOptions & {withFileTypes: true; lazy: true},
): AsyncGenerator<Dirent>;
(pattern: string | readonly string[], options: GlobOptions & {withFileTypes: true; lazy?: false}): Promise<Dirent[]>;
(
pattern: string | readonly string[],
options: GlobOptions & {withFileTypes: true},
): Promise<Dirent[]> | AsyncGenerator<Dirent>;
(pattern: string | readonly string[], options: GlobOptions & {lazy: true}): AsyncGenerator<string>;
(
pattern: string | readonly string[],
options?: GlobOptions,
): Promise<string[]> | Promise<Dirent[]> | AsyncGenerator<string> | AsyncGenerator<Dirent>;
}
/**
* Maps {@link CopyFileOptions} (including legacy `ncp` fields) to `fs.cp` options.
*/
@@ -237,12 +273,31 @@ export const fs = {
which,
/**
* Given a glob pattern, resolve with list of files matching that pattern.
* @see https://github.com/isaacs/node-glob
* Given a glob pattern, resolves to an array of matching paths (or `Dirent`s if `withFileTypes` is set).
* Pass `lazy: true` to get an async generator that yields matches one at a time instead.
*
* Unlike the `glob` npm package, this does NOT follow symlinks when expanding wildcard path
* segments. Prefer a manual `readdir`-based walk over a glob pattern for callers that must
* traverse through symlinks.
*/
glob(pattern: string, options?: GlobOptions): Promise<string[]> {
return Promise.resolve((options ? glob(pattern, options) : glob(pattern)) as Promise<string[]>);
},
glob: ((pattern: string | readonly string[], options: GlobOptions = {}) => {
const {cwd, withFileTypes, absolute, lazy} = options;
async function* generate(): AsyncGenerator<string | Dirent> {
for await (const entry of fsPromises.glob(pattern, {cwd, withFileTypes})) {
yield absolute && !withFileTypes ? path.resolve(cwd ?? process.cwd(), entry as string) : entry;
}
}
if (lazy) {
return generate();
}
return (async () => {
const items: (string | Dirent)[] = [];
for await (const item of generate()) {
items.push(item);
}
return items;
})();
}) as GlobFn,
/** Sanitize a filename. @see https://github.com/parshap/node-sanitize-filename */
sanitizeName: sanitize,
-1
View File
@@ -57,7 +57,6 @@
"archiver": "8.0.0",
"asyncbox": "6.4.2",
"axios": "1.19.0",
"glob": "13.0.6",
"klaw": "4.1.0",
"lockfile": "1.0.4",
"mime-types": "3.0.2",
+8
View File
@@ -169,6 +169,14 @@ describe('fs', {timeout: TEST_TIMEOUT}, function () {
assert.ok(Array.isArray(tests));
assert.ok(tests.length > 2);
});
it('glob() with lazy', async function () {
const glob = '*.spec.js';
const tests: string[] = [];
for await (const test of fs.glob(glob, {cwd: import.meta.dirname, lazy: true})) {
tests.push(test);
}
assert.ok(tests.length > 2);
});
describe('walkDir()', function () {
it('walkDir recursive', async function () {
+2 -3
View File
@@ -44,9 +44,8 @@ async function writeJson(filepath, data) {
* @returns {Promise<string[]>} An array of package directory paths.
*/
async function getPackageDirs() {
return (await fs.glob('*/package.json', {cwd: ROOT_PACKAGES_DIR, absolute: true}))
.map((pkgJsonPath) => path.dirname(pkgJsonPath))
.sort();
const pkgJsonPaths = await fs.glob('*/package.json', {cwd: ROOT_PACKAGES_DIR, absolute: true});
return pkgJsonPaths.map((pkgJsonPath) => path.dirname(pkgJsonPath)).sort();
}
/**