fix: consolidate isolated node/link attributes

[wraithgar]

We are making fake versions of nodes and links in isolated mode. With this we are at least being consistent w/ the default attributes. For example isStoreLink was not on workspace links, but it is now.

@manzoorwanijk Heads up here. This is step one of the "fake Node and Link" cleanup I was talking about.

[wraithgar]

This is the only "new" code path here and we are not testing a linked root tree in isolated mode.

[manzoorwanijk]

@manzoorwanijk Heads up here. This is step one of the "fake Node and Link" cleanup I was talking about.

Thank you for the heads up. For now, I would want to backport #9066 to v10.

[wraithgar]

Yeah we can do another backport PR next week. I'm still iterating on this, I have more consolidation I'm doing now.

[manzoorwanijk]

Yeah we can do another backport PR next week. I'm still iterating on this, I have more consolidation I'm doing now.

Sounds good. Thank you.

[wraithgar]

@owlstronaut this is ready for review now.

[wraithgar]

@manzoorwanijk given #9064 I think I see now why there is a "top" attribute here that seems out of place. It's left over from before the add omits to trash list refactor.

We'll want to be sure to account for that either here or when we backport this refactor to v10.

[owlstronaut]

Hmm, won't workspace nodes pick this up?

[wraithgar]

These are links not nodes.

[owlstronaut]

Right, sorry, that's what I meant -

cli/workspaces/arborist/lib/arborist/isolated-reifier.js

Line 294 in e65ce49

const wsLink = new IsolatedLink({

is where I'm worried about the workspace links being identified as store links

[wraithgar]

This is purely for the _diffTree, and it helps match nodes to their actual place on disk. Workspaces ARE links.

[owlstronaut]

Agreed they are links, but I think the isStoreLink returning true will be a change in behavior for them. There is some deduplication code in rebuild

cli/workspaces/arborist/lib/arborist/rebuild.js

Lines 107 to 115 in 3b96929

	for (const node of nodes) {
	if (node.isStoreLink) {
	storeNodes.add(node)
	} else if (node.isLink) {
	linkNodes.add(node)
	} else {
	depNodes.add(node)
	}
	}

. Workspace links will change their 'type'. This code seems to prevent lifecycle scripts from running twice?

[wraithgar]

Ok I'll go make that a this attribute and only pass it in and set it where it was explicit before.

[wraithgar]

Those .top attributes end up being used at least once in reify's isolated mode path so I put them back. We can deal with it later.

[owlstronaut]

woohoo! This is awesome