Skip to content

Conversation

@wraithgar
Copy link
Member

This is a continuation of #7237. The eventual goal is to remove the mixin approach, as its original goals are no longer being met.

  • constructor logic was consolidated. It takes place in the main Arborist constructor when possible, allowing us to see all of the constructor at once and find any duplications or problems. It's evident that our approach to options/this.options needs some attention.
  • many symbol.for entries were moved to private methods or attributes once code was consolidated.
  • Several small single-use methods were inlined into the code that called them. In many cases this prevented re-pulling variables from this.
  • inline both #resolveLinks methods. There was one in build-ideal-tree and one in load-virtual, each one doing different things. There was no overlap in logic.
  • roll load-virtual up into build-ideal-tree. These shared internal symbols, a good sign that their concerns are not separated.
  • remove unused param from cal to #linkFromSpec. The function is not expecting a fourth parameter
  • skip optional check for optional-set. Every time we call this we have already done this check. Unit tests were masking this dead code.
  • roll rebuild up into reify. Allowed for more symbol conversion to private methods/attribute.
  • roll load-actual up into build-ideal-tree. same story as load-virtual.
  • move reifyPackages to private method and remove unused private attributes. chore: fix pruner and reify tests for optional peer deps #8540 removed the bad tests that were hooking into reifyPackages, and another recent PR removed the #dryRun and #savePrefix code.

There are a lot of small changes here, broken up into individual commits for easier review if needed. We'll squash to land it though.

@wraithgar wraithgar requested a review from a team as a code owner October 15, 2025 16:09
}
// normalize trailing slash
const registry = options.registry || 'https://registry.npmjs.org'
options.registry = this.registry = registry.replace(/\/+$/, '') + '/'

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
@wraithgar
Copy link
Member Author

We'll have to break this up to see where those yaml warnings come from.

@wraithgar wraithgar closed this Oct 15, 2025
@wraithgar wraithgar mentioned this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant