fix: dep flag calculation

[liamcmitchell]

• reverts pruning added in fix: prune optional peer dependencies that are no longer explicitly depended on #8431, which incorrectly prunes deps flagged as peer and optional - these flags don't mean that this node is an optional peer!
• reverts much of fix: corrects peer dependency flag propagation #8579, which I think mistakenly changed peer dep calculation logic
• rewrites calcDepFlags
• adds logic to avoid unsetting extraneous when following optional peer edges (how fix: prune optional peer dependencies that are no longer explicitly depended on #8431, should have been fixed)
• updates my prev fix to avoid looking for missing optional peer deps (if ((!edge.to && edge.type !== 'peerOptional') || !edge.valid) {)
• refactors dep flag unsetting and resetting into Node methods
• removes shake out Link target timing issue test, which was testing code removed a while back
• avoids omitting flakyselflink fixture when writing snapshots

Fixes #8535

[liamcmitchell]

A few more words on dep flags...

Their intention is documented here: https://github.com/npm/cli/blob/latest/workspaces/arborist/README.md#package-dependency-flags

relevant when pruning nodes from the tree

Pruning means both the prune command which removes extraneous nodes and the production/omit/include flags which filter a subset of deps.

The optional flag is also used to decide if a dep installation failure can be ignored but because of the intended use in pruning, the optional flag might not always allow failure as expected.

Consider the following

tree (dev A, optional B)
+-- A (prod C) dev: true
+-- B (optional C) optional: true
+-- C dev: false, optional: false, devOptional: true

C is not dev or optional because it won't be pruned as either. It is devOptional because it will be pruned if both dev and optional are omitted together.

If C installation fails in npm install --omit=dev... the failure will be fatal because C is not optional - even though the only prod dependent wasn't installed.

Maybe another edge case that can be fixed by recalculating dep flags again after diffing? Or is it before diffing? After we check if a package is omitted but before we check if it is installable?

I think there are a lot of subtle bugs caused by this fragile pre-calculation of dep flags and trying to keep it in sync.

Why are dep flags pre-calculated at all? We only need them when pruning or handling install failures. Could this be done lazily in diff/reify?

Why are dep flags persisted to lockfiles and not just re-calculated? Performance? Curiosity? Preserving inconsistent dep flag calculations across different npm versions? 🤔

[wraithgar]

Why are dep flags persisted to lockfiles and not just re-calculated?

I believe the reason is so that things like --omit=dev work from the shrinkwrap and not from re-calculating the tree.

ref:
npm/npm@1eabcd1
and
npm/npm#10073

Why are dep flags pre-calculated at all? We only need them when pruning or handling install failures. Could this be done lazily in diff/reify?

Do we have enough info from a shrinkwrap install to still calculate all of this without the flags IN the shrinkwrap? Does this question even make sense anymore (i.e. are we currently still pulling in the same info anyways that we would if the flags weren't in the shrinkwrap?)

These are all good questions you have. I think this is a case of things making sense at the time, and the software having iterated enough over time that we can rethink some decisions.

Removing these flags from the lockfile is a pretty significant breaking change, however. Making changes that aren't breaking would be the challenge here. Can we still persist them but have the new code ignore them?