An Old Bug #
Recently, I happened across weird line in
read-package-tree
while reading through the code to see where I might get started
implementing Workspaces for the npm CLI.
At the time, I was so deep in the flow of reading code and tracing flows
through various parts of the system, it didn't strike me how important it
was. I just thought "oh, that's obviously wrong" and fixed
it
without a second thought. When I tried to integrate my changes back to the
mainline CLI with read-package-tree version 5.3.0, however, I realized
what I'd found.
It might not seem weird at first, so I'll provide some context.
The read-package-tree module reads the tree of packages in a
node_modules folder. It doesn't do dependency resolution, or figure out
how that tree needs to be modified during an install or update, but it
does provide the basic data structure that is subsequently modified
within the npm CLI installer codebase. Most importantly, it loads Node
objects with parent and children links on them.
Node.js's module loader has a subtle and important property to it, which
makes a lot of npm's magic possible, and has made it such a good choice
over the years as a way to create CLI utilities. When a module in Node
calls require('foo'), Node looks up through the node_modules hierarchy
to find the foo module. However, importantly, when a package is
symlinked into another location, the module resolution process runs based
on the target location, not the link location.
This is why, for example, you can have a symlink from /usr/local/bin/npm
that points to /usr/local/lib/node_modules/npm/bin/npm-cli.js, but the
program doesn't have to be updated to carry all its dependencies along with
it. This is how you can have multiple different versions of a dependency
loaded in the same Node program at the same time, providing a release valve
on the dependency constraint solver and thus avoiding dependency hell.
So back to this weird line. In the context of that file, this bit of code is loading up the children of a given node. In order to match Node's module resolution algorithm, that should be reading the target and then loading the children there, rather than treating the link as the parent.
In the mindset of general code cleanup, I modified it without even really noticing and then mistakenly let it slip past into a completely unrelated commit.
When I tried to integrate read-package-tree (with this change) into the
npm CLI, a bunch of tests broke in really bizarre ways. That's when it hit
me what was going on, and a lot of strange CLI behavior over the years
started to make a ton of sense. Like, how occasionally a manually
symlinked module would result in its dependencies being stripped away or
mutated in some way that seemed to make no sense. (I'm sure most people
don't do this, but as someone who is probably too comfortable with module
systems, I often mess with my projects with reckless abandon.)
Prior to npm v3, there was no deduplication by default, and installation happened in a very naive "run in parallel, fetch whatever is still needed at that level" kind of way. It was dumb, in a mostly good way, but the result was that occasionally you'd get 15 copies of something when 1 would do just fine. It also meant that a proper progress bar was impossible, since the installer had no way of knowing when it would be done. (Simple loop detection helped it know that eventually it would be done, but not when, exactly.)
Over the years, npm has had a lot of bugfixes and logic added to work around the issues that this one subtle mistake caused. The result of all that fixing, however, means that fixing the bug breaks a lot of stuff that works just fine today. The good news is that npm v7 will have Workspaces, and (once the installer is completely refactored into @npmcli/arborist), it'll be trivial to implement.
Out of idle curiosity, I tracked down where this bug had snuck in, though I
was pretty sure I already knew the answer. It dates back to the original
inception of the read-package-tree
module. It
really should have been caught and fixed in
db70482 but,
well, it wasn't.
Spotted my past self red-handed, messing with me as usual, sneaking bugs into my code. If I ever catch that guy, he'll have quite a few things to answer for.