From d2c7c33343d6253c42b41a0d2c5a964311eebcd4 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 21 Jun 2022 07:57:26 -0700 Subject: [PATCH] fix: cleanup --include-workspace-root p-map / map concurrency optimizing and refactoring other loops docs updates and more --- docs/content/commands/npm-query.md | 2 +- .../content/using-npm/dependency-selectors.md | 21 +- lib/commands/query.js | 79 +++--- lib/utils/query-selector-all-response.js | 30 --- package-lock.json | 7 +- .../test/lib/commands/query.js.test.cjs | 38 ++- .../test/lib/load-all-commands.js.test.cjs | 2 +- tap-snapshots/test/lib/npm.js.test.cjs | 2 +- test/lib/commands/query.js | 49 ++++ workspaces/arborist/lib/query-selector-all.js | 245 +++++++++--------- workspaces/arborist/package.json | 1 + .../arborist/test/query-selector-all.js | 44 ++-- 12 files changed, 307 insertions(+), 213 deletions(-) delete mode 100644 lib/utils/query-selector-all-response.js diff --git a/docs/content/commands/npm-query.md b/docs/content/commands/npm-query.md index c9636bdf3a332..061bf4ce42442 100644 --- a/docs/content/commands/npm-query.md +++ b/docs/content/commands/npm-query.md @@ -11,7 +11,7 @@ description: Dependency selector query ```bash -npm query +npm query ``` diff --git a/docs/content/using-npm/dependency-selectors.md b/docs/content/using-npm/dependency-selectors.md index 997a2459afb30..9c76355806b2f 100644 --- a/docs/content/using-npm/dependency-selectors.md +++ b/docs/content/using-npm/dependency-selectors.md @@ -20,16 +20,29 @@ The `npm query` commmand exposes a new dependency selector syntax (informed by & - there is no "type" or "tag" selectors (ex. `div, h1, a`) as a dependency/target is the only type of `Node` that can be queried - the term "dependencies" is in reference to any `Node` found in a `tree` returned by `Arborist` +#### Combinators + +- `>` direct descendant/child +- ` ` any descendant/child +- `~` sibling + #### Selectors - `*` universal selector - `#` dependency selector (equivalent to `[name="..."]`) - `#@` (equivalent to `[name=]:semver()`) - `,` selector list delimiter -- `.` class selector -- `:` pseudo class selector -- `>` direct decendent/child selector -- `~` sibling selector +- `.` dependency type selector +- `:` pseudo selector + +#### Dependency Type Selectors + +- `.prod` dependency found in the `dependencies` section of `package.json`, or is a child of said dependency +- `.dev` dependency found in the `devDependencies` section of `package.json`, or is a child of said dependency +- `.optional` dependency found in the `optionalDependencies` section of `package.json`, or has `"optional": true` set in its entry in the `peerDependenciesMeta` section of `package.json`, or a child of said dependency +- `.peer` dependency found in the `peerDependencies` section of `package.json` +- `.workspace` dependency found in the `workspaces` section of `package.json` +- `.bundled` dependency found in the `bundleDependencies` section of `package.json`, or is a child of said dependency #### Pseudo Selectors - [`:not()`](https://developer.mozilla.org/en-US/docs/Web/CSS/:not) diff --git a/lib/commands/query.js b/lib/commands/query.js index 2058124ab7918..6e72b49dfb58a 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -3,28 +3,30 @@ const { resolve } = require('path') const Arborist = require('@npmcli/arborist') const BaseCommand = require('../base-command.js') -const QuerySelectorAllResponse = require('../utils/query-selector-all-response.js') -// retrieves a normalized inventory -const convertInventoryItemsToResponses = inventory => { - const responses = [] - const responsesSeen = new Set() - for (const node of inventory) { - if (!responsesSeen.has(node.target.realpath)) { - const item = new QuerySelectorAllResponse(node) - responses.push(item) - responsesSeen.add(item.path) - } +class QuerySelectorItem { + constructor (node) { + // all enumerable properties from the target + Object.assign(this, node.target.package) + + // append extra info + this.pkgid = node.target.pkgid + this.location = node.target.location + this.path = node.target.path + this.realpath = node.target.realpath + this.resolved = node.target.resolved + this.isLink = node.target.isLink + this.isWorkspace = node.target.isWorkspace } - return responses } class Query extends BaseCommand { + #response = [] // response is the query response + #seen = new Set() // paths we've seen so we can keep response deduped + static description = 'Retrieve a filtered list of packages' static name = 'query' - static usage = [ - '', - ] + static usage = [''] static ignoreImplicitWorkspace = false @@ -35,9 +37,13 @@ class Query extends BaseCommand { 'include-workspace-root', ] - async exec (args, workspaces) { - const globalTop = resolve(this.npm.globalDir, '..') - const where = this.npm.config.get('global') ? globalTop : this.npm.prefix + get parsedResponse () { + return JSON.stringify(this.#response, null, 2) + } + + async exec (args) { + // one dir up from wherever node_modules lives + const where = resolve(this.npm.dir, '..') const opts = { ...this.npm.flatOptions, path: where, @@ -45,33 +51,42 @@ class Query extends BaseCommand { const arb = new Arborist(opts) const tree = await arb.loadActual(opts) const items = await tree.querySelectorAll(args[0]) - const res = - JSON.stringify(convertInventoryItemsToResponses(items), null, 2) + this.buildResponse(items) - return this.npm.output(res) + this.npm.output(this.parsedResponse) } async execWorkspaces (args, filters) { await this.setWorkspaces(filters) - const result = new Set() const opts = { ...this.npm.flatOptions, path: this.npm.prefix, } const arb = new Arborist(opts) const tree = await arb.loadActual(opts) - for (const [, workspacePath] of this.workspaces.entries()) { - this.prefix = workspacePath - const [workspace] = await tree.querySelectorAll(`.workspace:path(${workspacePath})`) - const res = await workspace.querySelectorAll(args[0]) - const converted = convertInventoryItemsToResponses(res) - for (const item of converted) { - result.add(item) + for (const workspacePath of this.workspacePaths) { + let items + if (workspacePath === tree.root.path) { + // include-workspace-root + items = await tree.querySelectorAll(args[0]) + } else { + const [workspace] = await tree.querySelectorAll(`.workspace:path(${workspacePath})`) + items = await workspace.querySelectorAll(args[0]) + } + this.buildResponse(items) + } + this.npm.output(this.parsedResponse) + } + + // builds a normalized inventory + buildResponse (items) { + for (const node of items) { + if (!this.#seen.has(node.target.realpath)) { + const item = new QuerySelectorItem(node) + this.#response.push(item) + this.#seen.add(item.realpath) } } - // when running in workspaces names, make sure to key by workspace - // name the results of each value retrieved in each ws - this.npm.output(JSON.stringify([...result], null, 2)) } } diff --git a/lib/utils/query-selector-all-response.js b/lib/utils/query-selector-all-response.js deleted file mode 100644 index 56208a2d41689..0000000000000 --- a/lib/utils/query-selector-all-response.js +++ /dev/null @@ -1,30 +0,0 @@ -'use strict' - -class QuerySelectorAllResponse { - #node = null - - constructor (node) { - const { - location, - path, - realpath, - resolved, - isLink, - isWorkspace, - pkgid, - } = node.target - - Object.assign(this, node.target.package) - - // append extra info - this.pkgid = pkgid - this.location = location - this.path = path - this.realpath = realpath - this.resolved = resolved - this.isLink = isLink - this.isWorkspace = isWorkspace - } -} - -module.exports = QuerySelectorAllResponse diff --git a/package-lock.json b/package-lock.json index ee7af5ad0505f..bc9089d12ef67 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5549,8 +5549,9 @@ }, "node_modules/p-map": { "version": "4.0.0", + "resolved": "https://registry.npmjs.org/p-map/-/p-map-4.0.0.tgz", + "integrity": "sha512-/bjOqmgETBYB5BoEeGVea8dmvHb2m9GLy1E9W43yeyfP6QQCZGFNa+XRceJEuDB6zqr+gKpIAmlLebMpykw/MQ==", "inBundle": true, - "license": "MIT", "dependencies": { "aggregate-error": "^3.0.0" }, @@ -10055,6 +10056,7 @@ "npm-pick-manifest": "^7.0.0", "npm-registry-fetch": "^13.0.0", "npmlog": "^6.0.2", + "p-map": "^4.0.0", "pacote": "^13.6.1", "parse-conflict-json": "^2.0.1", "proc-log": "^2.0.0", @@ -10783,6 +10785,7 @@ "npm-pick-manifest": "^7.0.0", "npm-registry-fetch": "^13.0.0", "npmlog": "^6.0.2", + "p-map": "4.0.0", "pacote": "^13.6.1", "parse-conflict-json": "^2.0.1", "proc-log": "^2.0.0", @@ -13939,6 +13942,8 @@ }, "p-map": { "version": "4.0.0", + "resolved": "https://registry.npmjs.org/p-map/-/p-map-4.0.0.tgz", + "integrity": "sha512-/bjOqmgETBYB5BoEeGVea8dmvHb2m9GLy1E9W43yeyfP6QQCZGFNa+XRceJEuDB6zqr+gKpIAmlLebMpykw/MQ==", "requires": { "aggregate-error": "^3.0.0" } diff --git a/tap-snapshots/test/lib/commands/query.js.test.cjs b/tap-snapshots/test/lib/commands/query.js.test.cjs index ee87fd3b32233..c64d20f822bb0 100644 --- a/tap-snapshots/test/lib/commands/query.js.test.cjs +++ b/tap-snapshots/test/lib/commands/query.js.test.cjs @@ -13,8 +13,8 @@ exports[`test/lib/commands/query.js TAP global > should return global package 1` "_id": "lorem@2.0.0", "pkgid": "lorem@2.0.0", "location": "node_modules/lorem", - "path": "{CWD}/test/lib/commands/tap-testdir-query-global/global/lib/node_modules/lorem", - "realpath": "{CWD}/test/lib/commands/tap-testdir-query-global/global/lib/node_modules/lorem", + "path": "{CWD}/test/lib/commands/tap-testdir-query-global/global/node_modules/lorem", + "realpath": "{CWD}/test/lib/commands/tap-testdir-query-global/global/node_modules/lorem", "resolved": null, "isLink": false, "isWorkspace": false @@ -22,6 +22,40 @@ exports[`test/lib/commands/query.js TAP global > should return global package 1` ] ` +exports[`test/lib/commands/query.js TAP include-workspace-root > should return workspace object and root object 1`] = ` +[ + { + "name": "project", + "workspaces": [ + "c" + ], + "dependencies": { + "a": "^1.0.0", + "b": "^1.0.0" + }, + "pkgid": "project@", + "location": "", + "path": "{CWD}/test/lib/commands/tap-testdir-query-include-workspace-root/prefix", + "realpath": "{CWD}/test/lib/commands/tap-testdir-query-include-workspace-root/prefix", + "resolved": null, + "isLink": false, + "isWorkspace": false + }, + { + "name": "c", + "version": "1.0.0", + "_id": "c@1.0.0", + "pkgid": "c@1.0.0", + "location": "c", + "path": "{CWD}/test/lib/commands/tap-testdir-query-include-workspace-root/prefix/c", + "realpath": "{CWD}/test/lib/commands/tap-testdir-query-include-workspace-root/prefix/c", + "resolved": null, + "isLink": false, + "isWorkspace": true + } +] +` + exports[`test/lib/commands/query.js TAP linked node > should return linked node res 1`] = ` [ { diff --git a/tap-snapshots/test/lib/load-all-commands.js.test.cjs b/tap-snapshots/test/lib/load-all-commands.js.test.cjs index de34bd81e5c04..a50b1845bc3b4 100644 --- a/tap-snapshots/test/lib/load-all-commands.js.test.cjs +++ b/tap-snapshots/test/lib/load-all-commands.js.test.cjs @@ -687,7 +687,7 @@ exports[`test/lib/load-all-commands.js TAP load each command query > must match Retrieve a filtered list of packages Usage: -npm query +npm query Options: [-g|--global] diff --git a/tap-snapshots/test/lib/npm.js.test.cjs b/tap-snapshots/test/lib/npm.js.test.cjs index c3a7b8fc80608..3fc8c503364b1 100644 --- a/tap-snapshots/test/lib/npm.js.test.cjs +++ b/tap-snapshots/test/lib/npm.js.test.cjs @@ -739,7 +739,7 @@ All commands: query Retrieve a filtered list of packages Usage: - npm query + npm query Options: [-g|--global] diff --git a/test/lib/commands/query.js b/test/lib/commands/query.js index d4e98f28c69b3..9b7e5256444d5 100644 --- a/test/lib/commands/query.js +++ b/test/lib/commands/query.js @@ -7,6 +7,8 @@ t.cleanSnapshot = (str) => { .replace(/\r\n/g, '\n') return normalizePath(str) .replace(new RegExp(normalizePath(process.cwd()), 'g'), '{CWD}') + // normalize between windows and posix + .replace(new RegExp('lib/node_modules', 'g'), 'node_modules') } t.test('simple query', async t => { @@ -72,6 +74,43 @@ t.test('workspace query', async t => { t.matchSnapshot(joinedOutput(), 'should return workspace object') }) +t.test('include-workspace-root', async t => { + const { npm, joinedOutput } = await loadMockNpm(t, { + config: { + 'include-workspace-root': true, + workspaces: ['c'], + }, + prefixDir: { + node_modules: { + a: { + name: 'a', + version: '1.0.0', + }, + b: { + name: 'b', + version: '^2.0.0', + }, + c: t.fixture('symlink', '../c'), + }, + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + }), + }, + 'package.json': JSON.stringify({ + name: 'project', + workspaces: ['c'], + dependencies: { + a: '^1.0.0', + b: '^1.0.0', + }, + }), + }, + }) + await npm.exec('query', [':scope'], ['c']) + t.matchSnapshot(joinedOutput(), 'should return workspace object and root object') +}) t.test('linked node', async t => { const { npm, joinedOutput } = await loadMockNpm(t, { prefixDir: { @@ -101,7 +140,17 @@ t.test('global', async t => { config: { global: true, }, + // This is a global dir that works in both windows and non-windows, that's + // why it has two node_modules folders globalPrefixDir: { + node_modules: { + lorem: { + 'package.json': JSON.stringify({ + name: 'lorem', + version: '2.0.0', + }), + }, + }, lib: { node_modules: { lorem: { diff --git a/workspaces/arborist/lib/query-selector-all.js b/workspaces/arborist/lib/query-selector-all.js index e357a660d0258..0175204782a8c 100644 --- a/workspaces/arborist/lib/query-selector-all.js +++ b/workspaces/arborist/lib/query-selector-all.js @@ -1,22 +1,27 @@ 'use strict' +// TODO how many of these async functions do async operations? + const { resolve } = require('path') const { parser, arrayDelimiter } = require('@npmcli/query') const localeCompare = require('@isaacs/string-locale-compare')('en') const npa = require('npm-package-arg') const minimatch = require('minimatch') const semver = require('semver') +const pMap = require('p-map') + +// default concurrency when mapping over tree objects +const MAP_CONCURRENCY = 10 -// handle results for parsed query asts, results are stored in a map that -// has a key that points to each ast selector node and stores the resulting -// array of arborist nodes as its value, that is essential to how we handle -// multiple query selectors, e.g: `#a, #b, #c` <- 3 diff ast selector nodes +// handle results for parsed query asts, results are stored in a map that has a +// key that points to each ast selector node and stores the resulting array of +// arborist nodes as its value, that is essential to how we handle multiple +// query selectors, e.g: `#a, #b, #c` <- 3 diff ast selector nodes class Results { - #results = null + #results = new Map() #currentAstSelector = null constructor (rootAstNode) { - this.#results = new Map() this.#currentAstSelector = rootAstNode.nodes[0] } @@ -33,17 +38,11 @@ class Results { this.#results.set(this.#currentAstSelector, value) } - // when collecting results to a root astNode, we traverse the list of - // child selector nodes and collect all of their resulting arborist nodes - // into a single/flat Set of items, this ensures we also deduplicate items + // when collecting results to a root astNode, we traverse the list of child + // selector nodes and collect all of their resulting arborist nodes into a + // single/flat Set of items, this ensures we also deduplicate items collect (rootAstNode) { - const acc = new Set() - for (const n of rootAstNode.nodes) { - for (const node of this.#results.get(n)) { - acc.add(node) - } - } - return acc + return new Set(rootAstNode.nodes.flatMap(n => this.#results.get(n))) } } @@ -78,10 +77,10 @@ const retrieveNodesFromParsedAst = async ({ return (String(pkg[attribute] || '').match(/\w+/g) || []).includes(value) }, '*=' ({ attribute, value, pkg }) { - return String(pkg[attribute] || '').indexOf(value) > -1 + return String(pkg[attribute] || '').includes(value) }, '|=' ({ attribute, value, pkg }) { - return String(pkg[attribute] || '').split('-')[0] === value + return String(pkg[attribute] || '').startsWith(`${value}-`) }, '^=' ({ attribute, value, pkg }) { return String(pkg[attribute] || '').startsWith(value) @@ -90,43 +89,52 @@ const retrieveNodesFromParsedAst = async ({ return String(pkg[attribute] || '').endsWith(value) }, })) - const classesMap = new Map(Object.entries({ - '.prod' (prevResults) { - return Promise.resolve(prevResults.filter(node => - [...node.edgesIn].some(edge => edge.prod))) + const depTypesMap = new Map(Object.entries({ + async '.prod' (prevResults) { + return prevResults.filter(node => + [...node.edgesIn].some(edge => edge.prod) + ) }, - '.dev' (prevResults) { - return Promise.resolve(prevResults.filter(node => - [...node.edgesIn].some(edge => edge.dev))) + async '.dev' (prevResults) { + return prevResults.filter(node => + [...node.edgesIn].some(edge => edge.dev) + ) }, - '.optional' (prevResults) { - return Promise.resolve(prevResults.filter(node => - [...node.edgesIn].some(edge => edge.optional))) + async '.optional' (prevResults) { + return prevResults.filter(node => + [...node.edgesIn].some(edge => edge.optional) + ) }, - '.peer' (prevResults) { - return Promise.resolve(prevResults.filter(node => - [...node.edgesIn].some(edge => edge.peer))) + async '.peer' (prevResults) { + return prevResults.filter(node => + [...node.edgesIn].some(edge => edge.peer) + ) }, - '.workspace' (prevResults) { - return Promise.resolve( - prevResults.filter(node => node.isWorkspace)) + async '.workspace' (prevResults) { + return prevResults.filter(node => + node.isWorkspace + ) }, - '.bundled' (prevResults) { - return Promise.resolve( - prevResults.filter(node => node.inBundle)) + async '.bundled' (prevResults) { + return prevResults.filter(node => + node.inBundle + ) }, })) - const hasParent = (node, compareNodes) => { + // checks if a given node has a direct parent in any of the nodes provided in + // the compare nodes array + const hasParent = async (node, compareNodes) => { if (parentCache.has(node) && parentCache.get(node).has(compareNodes)) { - return Promise.resolve(true) + return true } const parentFound = compareNodes.some(compareNode => { // follows logical parent for link anscestors - return (node.isTop && node.resolveParent) === compareNode || + if (node.isTop && (node.resolveParent === compareNode)) { + return true + } // follows edges-in to check if they match a possible parent - [...node.edgesIn].some(edge => - edge && edge.from === compareNode) + return [...node.edgesIn].some(edge => edge && edge.from === compareNode) }) if (parentFound) { @@ -136,63 +144,71 @@ const retrieveNodesFromParsedAst = async ({ parentCache.get(node).add(compareNodes) } - return Promise.resolve(parentFound) + return parentFound } - // checks if a given node is a descendant of any - // of the nodes provided in the compare nodes array + // checks if a given node is a descendant of any of the nodes provided in the + // compare nodes array const hasAscendant = async (node, compareNodes) => { - const hasP = await hasParent(node, compareNodes) - if (hasP) { + if (await hasParent(node, compareNodes)) { return true } - const lookupEdgesIn = async (node) => { - const edgesIn = [...node.edgesIn] - const p = await Promise.all( - edgesIn.map(edge => - edge && edge.from && hasAscendant(edge.from, compareNodes)) - ) - return edgesIn.some((edge, index) => p[index]) + if (node.isTop && node.resolveParent) { + return hasAscendant(node.resolveParent, compareNodes) } - const ancestorFound = (node.isTop && node.resolveParent) - ? await hasAscendant(node.resolveParent, compareNodes) - : await lookupEdgesIn(node) - - return ancestorFound + const hasEdgesIn = await pMap( + node.edgesIn, + async edge => edge && edge.from && hasAscendant(edge.from, compareNodes), + { concurrency: MAP_CONCURRENCY } + ) + return hasEdgesIn.includes(true) } const combinatorsMap = new Map(Object.entries({ + // direct descendant async '>' (prevResults, nextResults) { - const p = await Promise.all( - nextResults.map(i => hasParent(i, prevResults)) - ) - return nextResults.filter((nextItem, index) => p[index]) + // Return any node in nextResults that has a parent in prevResults + const result = [] + await pMap(nextResults, async node => { + if (await hasParent(node, prevResults)) { + result.push(node) + } + }, { concurrency: MAP_CONCURRENCY }) + return result }, + // any descendant async ' ' (prevResults, nextResults) { - const p = await Promise.all( - nextResults.map(i => hasAscendant(i, prevResults)) - ) - return nextResults.filter((nextItem, index) => p[index]) + // Return any node in nextResults that has an ascendant in prevResults + const result = [] + await pMap(nextResults, async node => { + if (await hasAscendant(node, prevResults)) { + result.push(node) + } + }, { concurrency: MAP_CONCURRENCY }) + return result }, + // sibling async '~' (prevResults, nextResults) { - const p = await Promise.all(nextResults.map(nextItem => { - const seenNodes = new Set() - const possibleParentNodes = - prevResults - .flatMap(node => { - seenNodes.add(node) - return [...node.edgesIn] - }) - .map(edge => edge.from) - .filter(Boolean) - - return !seenNodes.has(nextItem) && - hasParent(nextItem, [...possibleParentNodes]) - })) - return nextResults.filter((nextItem, index) => p[index]) + // Return any node in nextResults that is a sibling of (aka shares a + // parent with) a node in prevResults + const parentNodes = new Set() // Parents of everything in prevResults + for (const node of prevResults) { + for (const edge of node.edgesIn) { + // edge.from always exists cause it's from another node's edgesIn + parentNodes.add(edge.from) + } + } + const result = [] + await pMap(nextResults, async node => { + if (!prevResults.includes(node) && await hasParent(node, [...parentNodes])) { + result.push(node) + } + }, { concurrency: MAP_CONCURRENCY }) + return result }, })) + const pseudoMap = new Map(Object.entries({ async ':attr' () { const initialItems = getInitialItems() @@ -226,23 +242,13 @@ const retrieveNodesFromParsedAst = async ({ // all its indexes testing for possible objects that may eventually // hold more keys specified in a selector if (prop === arrayDelimiter) { - const newObjs = [] - for (const obj of objs) { - if (Array.isArray(obj)) { - obj.forEach((i, index) => { - newObjs.push(obj[index]) - }) - } else { - newObjs.push(obj) - } - } - objs = newObjs + objs = objs.flat() continue } else { // otherwise just maps all currently found objs // to the next prop from the lookup properties list, // filters out any empty key lookup - objs = objs.map(obj => obj[prop]).filter(Boolean) + objs = objs.flatMap(obj => obj[prop] || []) } // in case there's no property found in the lookup @@ -344,26 +350,26 @@ const retrieveNodesFromParsedAst = async ({ : getInitialItems() }, async ':type' () { + // TODO do we really have to iterate three times through the items? return currentAstNode.typeValue ? getInitialItems() .flatMap(node => [...node.edgesIn]) .filter(edge => npa(`${edge.name}@${edge.spec}`).type === currentAstNode.typeValue) - .map(edge => edge.to) - .filter(Boolean) + .flatMap(edge => edge.to) : getInitialItems() }, })) // retrieves the initial items to which start the filtering / matching - // for most of the different types of recognized ast nodes, e.g: class, - // id, *, etc in different contexts we need to start with the current list - // of filtered results, for example a query for `.workspace` actually - // means the same as `*.workspace` so we want to start with the full - // inventory if that's the first ast node we're reading but if it appears - // in the middle of a query it should respect the previous filtered - // results, combinators are a special case in which we always want to - // have the complete inventory list in order to use the left-hand side - // ast node as a filter combined with the element on its right-hand side + // for most of the different types of recognized ast nodes, e.g: class (aka + // depType), id, *, etc in different contexts we need to start with the + // current list of filtered results, for example a query for `.workspace` + // actually means the same as `*.workspace` so we want to start with the full + // inventory if that's the first ast node we're reading but if it appears in + // the middle of a query it should respect the previous filtered results, + // combinators are a special case in which we always want to have the + // complete inventory list in order to use the left-hand side ast node as a + // filter combined with the element on its right-hand side const getInitialItems = () => { const firstParsed = currentAstNode.parent.nodes[0] === currentAstNode && currentAstNode.parent.parent.type === 'root' @@ -419,16 +425,16 @@ const retrieveNodesFromParsedAst = async ({ results.currentResult = await processPendingCombinator(prevResults, nextResults) } - const classType = async () => { - const classFn = classesMap.get(String(currentAstNode)) - if (!classFn) { + const depType = async () => { + const depTypeFn = depTypesMap.get(String(currentAstNode)) + if (!depTypeFn) { throw Object.assign( - new Error(`\`${String(currentAstNode)}\` is not a supported class.`), - { code: 'EQUERYNOCLASS' } + new Error(`\`${String(currentAstNode)}\` is not a supported dependency type.`), + { code: 'EQUERYNODEPTYPE' } ) } const prevResults = results.currentResult - const nextResults = await classFn(getInitialItems()) + const nextResults = await depTypeFn(getInitialItems()) results.currentResult = await processPendingCombinator(prevResults, nextResults) } @@ -449,8 +455,8 @@ const retrieveNodesFromParsedAst = async ({ if (!pseudoFn) { throw Object.assign( new Error(`\`${currentAstNode.value - }\` is not a supported pseudo-class.`), - { code: 'EQUERYNOPSEUDOCLASS' } + }\` is not a supported pseudo selector.`), + { code: 'EQUERYNOPSEUDO' } ) } const prevResults = results.currentResult @@ -473,11 +479,10 @@ const retrieveNodesFromParsedAst = async ({ await processPendingCombinator(prevResults, nextResults) } - // maps each of the recognized css selectors - // to a function that parses it + // maps each of the recognized css selectors to a function that parses it const retrieveByType = new Map(Object.entries({ attribute, - class: classType, + class: depType, combinator, id, pseudo, @@ -485,8 +490,8 @@ const retrieveNodesFromParsedAst = async ({ universal, })) - // walks through the parsed css query and update the - // current result after parsing / executing each ast node + // walks through the parsed css query and update the current result after + // parsing / executing each ast node const astNodeQueue = new Set() rootAstNode.walk((nextAstNode) => { astNodeQueue.add(nextAstNode) @@ -506,8 +511,8 @@ const retrieveNodesFromParsedAst = async ({ const querySelectorAll = async (targetNode, query) => { const rootAstNode = parser(query) - // results is going to be a Map in which its values are the - // resulting items returned for each parsed css ast selector + // results is going to be a Map in which its values are the resulting items + // returned for each parsed css ast selector const inventory = [...targetNode.root.inventory.values()] const res = await retrieveNodesFromParsedAst({ initialItems: inventory, diff --git a/workspaces/arborist/package.json b/workspaces/arborist/package.json index ff7eeffd318ed..9b342fcd58c14 100644 --- a/workspaces/arborist/package.json +++ b/workspaces/arborist/package.json @@ -27,6 +27,7 @@ "npm-pick-manifest": "^7.0.0", "npm-registry-fetch": "^13.0.0", "npmlog": "^6.0.2", + "p-map": "^4.0.0", "pacote": "^13.6.1", "parse-conflict-json": "^2.0.1", "proc-log": "^2.0.0", diff --git a/workspaces/arborist/test/query-selector-all.js b/workspaces/arborist/test/query-selector-all.js index 166a654820d78..0a504e676758f 100644 --- a/workspaces/arborist/test/query-selector-all.js +++ b/workspaces/arborist/test/query-selector-all.js @@ -178,28 +178,28 @@ t.test('query-selector-all', async t => { const emptyRes = await q(tree, '') t.same(emptyRes, [], 'empty query') - // missing pseudo-class + // missing pseudo selector t.rejects( q(tree, ':foo'), - { code: 'EQUERYNOPSEUDOCLASS' }, - 'should throw on missing pseudo-class' + { code: 'EQUERYNOPSEUDO' }, + 'should throw on missing pseudo selector' ) - // missing class + // missing depType t.rejects( q(tree, '.foo'), - { code: 'EQUERYNOCLASS' }, - 'should throw on missing class' + { code: 'EQUERYNODEPTYPE' }, + 'should throw on missing dep type' ) // missing attribute matcher on :attr t.rejects( q(tree, ':attr(foo, bar)'), { code: 'EQUERYATTR' }, - 'should throw on missing attribute matcher on :attr pseudo-class' + 'should throw on missing attribute matcher on :attr pseudo' ) - // :scope pseudo-class + // :scope pseudo const [nodeFoo] = await q(tree, '#foo') const scopeRes = await querySelectorAll(nodeFoo, ':scope') t.same(scopeRes, ['foo@2.2.2'], ':scope') @@ -212,12 +212,14 @@ t.test('query-selector-all', async t => { const runSpecParsing = async testCase => { for (const [selector, expected] of testCase) { - const res = await querySelectorAll(tree, selector) - t.same( - res, - expected, - selector - ) + t.test(selector, async t => { + const res = await querySelectorAll(tree, selector) + t.same( + res, + expected, + selector + ) + }) } } @@ -302,7 +304,7 @@ t.test('query-selector-all', async t => { [':missing', ['missing-dep@^1.0.0']], [':private', ['b@1.0.0']], - // :not pseudo-class + // :not pseudo [':not(#foo)', [ 'query-selector-all-tests@1.0.0', 'a@1.0.0', @@ -330,14 +332,14 @@ t.test('query-selector-all', async t => { 'b@1.0.0', ]], - // has pseudo-class + // has pseudo [':root > *:has(* > #bar@1.4.0)', ['foo@2.2.2']], ['*:has(* > #bar@1.4.0)', ['foo@2.2.2']], ['*:has(> #bar@1.4.0)', ['foo@2.2.2']], ['.workspace:has(> * > #lorem)', ['a@1.0.0']], ['.workspace:has(* #lorem, ~ #b)', ['a@1.0.0']], - // is pseudo-class + // is pseudo [':is(#a, #b) > *', ['bar@2.0.0', 'baz@1.0.0']], // TODO: ipsum is not empty but it's child is missing // so it doesn't return a result here @@ -350,7 +352,7 @@ t.test('query-selector-all', async t => { 'dasher@2.0.0', ]], - // type pseudo-class + // type pseudo [':type()', [ 'query-selector-all-tests@1.0.0', 'a@1.0.0', @@ -378,7 +380,7 @@ t.test('query-selector-all', async t => { ]], [':type(git)', []], - // path pseudo-class + // path pseudo [':path(node_modules/*)', [ 'abbrev@1.1.1', 'bar@2.0.0', @@ -410,7 +412,7 @@ t.test('query-selector-all', async t => { 'lorem@1.0.0', ]], - // semver pseudo-class + // semver pseudo [':semver()', [ 'query-selector-all-tests@1.0.0', 'a@1.0.0', @@ -461,7 +463,7 @@ t.test('query-selector-all', async t => { [':semver(=1.4.0)', ['bar@1.4.0']], [':semver(1.4.0 || 2.2.2)', ['foo@2.2.2', 'bar@1.4.0']], - // attr pseudo-class + // attr pseudo [':attr([name=dasher])', ['dasher@2.0.0']], [':attr(dependencies, [bar="^1.0.0"])', ['foo@2.2.2']], [':attr(dependencies, :attr([bar="^1.0.0"]))', ['foo@2.2.2']],