Skip to content
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

feat(query): Add command alongside package selector syntax #463

Closed
45 tasks done
Tracked by #489
darcyclarke opened this issue Mar 18, 2022 · 6 comments
Closed
45 tasks done
Tracked by #489

feat(query): Add command alongside package selector syntax #463

darcyclarke opened this issue Mar 18, 2022 · 6 comments
Assignees
Labels

Comments

@darcyclarke
Copy link
Contributor

darcyclarke commented Mar 18, 2022

Summary

Introduce a much needed top-level command & net-new capabilities for dependency selection/discovery.

Exit Criteria

  • Create new npm query command which parses a dependency selector & returns a NodeList
  • Add new .querySelectorAll() method to Arborist's Node class
  • Tests are written for new command
  • Documentation is written for new command & selector syntax

Selector Breakdown

Universal Selectors

  • * universal selector
  • #<name> dependency selector (equivalent to [name="..."])
  • #<name>@<version> (equivalent to [name=<name>]:semver(<version>))
  • , selector list delimiter
  • . class selector
  • : pseudo class selector
  • > direct decendent/child selector
  • ~ sibling selector

Attribute Selectors

  • [] attribute selector (ie. existence of attribute)
  • [attribute=value] attribute value is equivalant...
  • [attribute~=value] attribute value contains word...
  • [attribute*=value] attribute value contains string...
  • [attribute|=value] attribute value is equal to or starts with...
  • [attribute^=value] attribute value begins with...
  • [attribute$=value] attribute value ends with...

Pseudo Selectors

  • :not(<selector>)
  • :has(<selector>)
  • :is(<selector list>)
  • :root matches the root node/dependency
  • :scope matches node/dependency it was queried against
  • :empty when a dependency has no dependencies
  • :private when a dependency is private
  • :link when a dependency is linked
  • :extraneous when a dependency exists but is not defined as a dependency of any node
  • :invalid when a dependency version is out of its ancestors specified range
  • :missing when a dependency is not found on disk
  • :semver(<spec>) matching a valid node-semver spec
  • :deduped when a dependency has been deduped
  • :path(<path>) glob matching based on dependencies path relative to the project
  • :type(<type>) based on currently recognized types
  • :attr(<key>, <attribute selector>) see below

Array & Object Attribute Selectors

  • Object Selection:
/* return dependencies that have a `scripts.test` containing `"tap"` */
*:attr(scripts, [test~=tap])
  • Array Attribute Selections:
/* return dependencies that have a keyword that begins with "react" */
*:attr(keywords, [^=react])
  • Array match value Selection:
/* return dependencies that have the exact keyword "react" */
*:attr(keywords, [=react])
  • Array of Objects Selection:
/* returns contributors with the email "[email protected]" */
*:attr(contributors, [email=[email protected]])

Classes/Groups

  • .prod
  • .dev
  • .optional
  • .peer
  • .bundled
  • .workspace

Links & References

Stretch Goals

  • create new @npmcli/queryparser (or similar name) to manage syntactical nuances to the spec (ex. hashed package name & semver range de-sugaring)
  • :outdated() when a dependency has available updates (options for specifying the range, defaults to parent's dependent range)
  • :vulnerable when a dependency has a CVE
  • :override when a dependency is an override
    • Blocked: Arborist.loadActul() needs to be improved for ovrrides
    • Talk with @nlf
@darcyclarke darcyclarke mentioned this issue Mar 18, 2022
32 tasks
@darcyclarke darcyclarke changed the title New npm query command (alongside a package selector syntax) feat(query): Add command alongside a package selector syntax Mar 23, 2022
@darcyclarke darcyclarke changed the title feat(query): Add command alongside a package selector syntax feat(query): Add command alongside package selector syntax May 11, 2022
@darcyclarke darcyclarke mentioned this issue May 16, 2022
13 tasks
@wraithgar wraithgar assigned fritzy and unassigned ruyadorno Jun 14, 2022
@darcyclarke darcyclarke assigned wraithgar and unassigned fritzy Jun 23, 2022
@darcyclarke
Copy link
Contributor Author

darcyclarke commented Jun 23, 2022

High-level issues/investigation areas

  • :has() unbounded memory consumption issues... (update: I'll try to find the exact query that I ran to cause this - the latest version of the PR seems like it might have fixed this; w/ minimal testing npm query ":scope .dev" -ws just blew up in a similar fashion to what I've seen before)
    • I was coming across issues with :has() selectors that seemed to eat a tonne of resources; my guess is this has something to do with how we are querying the subset
  • :attr() arbitrary attribute selection does not seem to work as expected...
    • I was coming across several instances where I couldn't query arbitrary fields/metadata based on how we had spec'd that feature in the docs/above
    • :attr(<attribute selector>) should be matching against both key/value pairs as well as flat arrays with the corresponding value
    • :attr([<key>, ...], <attribute selector>) should be able to navigate through nested objects to then match a key/value pair or flat array value
    • :attr([<key>. ...], :attr(<attribute selector>)) should be able to navigate through nested objects & arrays to then match a key/value pair or flat array value
  • need to add support for attribute value case sensitivity (ref. https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors#syntax)
    • ex. [name="value" i] or [name="value" I]
    • ex. [name="value" s] or [name="value" S]

@wraithgar
Copy link
Member

Shouldn't values with spaces need to be quoted?

"Adding an i (or I) before the closing bracket causes the value to be compared case-insensitively (for characters within the ASCII range)."

@darcyclarke
Copy link
Contributor Author

darcyclarke commented Jun 23, 2022

@ljharb I've quoted them for clarity but as per @wraithgar's comment I do not believe they need to be in quoted based on what I've read (update: I've tested this & quotes aren't required).

@ljharb
Copy link

ljharb commented Jun 23, 2022

yes, indeed, i deleted my comment - i wasn't aware of that "suffix" feature.

@wraithgar
Copy link
Member

I think I fixed the has: bug while consolidating state, found an inconsistency in what was being passed around and fixed it, and the -w command that was melting my computer simply returns the correct result quickly now. Haven't pushed the change yet because I'm still consolidating state.

@wraithgar
Copy link
Member

@ruyadorno This code has been a treat to refactor. Every improvement I've made has either been a bugfix (which is inevitable in a codebase this big) or something that was high level and thus would have been inappropriate to even try to do on a first pass PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants