chore(swingset-vat): declare deps of testing tools

[turadg]

in pursuit of #5201

Description

The ava and @endo/ses-ava deps of @agoric/swingset-vat weren't declared in its package.json. In projects that imported that package and used tools/prepare-test-env-ava.js, they would get errors like this on test:

  Error: Cannot find package '@endo/ses-ava' imported from /opt/agoric/documentation/node_modules/@agoric/swingset-vat/tools/prepare-test-env-ava.js

I think this was due to a misconception about devDependencies. It doesn't mean dependencies used only for development; it means dependencies only for development of this package and not needed by any consumers of this package. tools files are exported by this package (and consumed in dapps) so for those modules to work the dependencies have to be available too.

This uses peerDependencies to ensure that the test environment has the right deps and that they're compatible with those at the package parent scope.

Security Considerations

--

Documentation Considerations

Some guidance around which kind of dep to use?
Maybe an audit of Eslint suppressions to see which are legitimate. Those that aren't we might disable.

Testing Considerations

CI

[mhofman]

Makes sense to me, though I think peerDependencies are a can of worms. Would it make sense to split swingset tools in its own package? That's probably a larger change. ping @FUDCo I believe you're the main maintainer of this code?

[turadg]

Would it make sense to split swingset tools in its own package? That's probably a larger change.

Yes, I think splitting it out would be better. I think it should go in a general @agoric/test-utils or something like that. Multiple packages export testing supports that don't need to be part of their semver commitments.

However let's not let perfect be the enemy of the good. This is an improvement that is already done and just needs approval.

[FUDCo]

Splitting these tools into their own package has a definite appeal. However, much of what they do is closely coupled to SwingSet internals (specifically, liveslots and how it sets up the relationship between the various bits of virtual object and stores machinery and liveslots' own GC machinery) since what they are doing is mocking the kernel side of liveslots and the database. If we were to make the tools their own package we'd have a package that depended heavily on deep imports from SwingSet, introducing some non-local dependencies that I think would not be good, or would require SwingSet to export stuff that under normal circumstances should not be exposed.

[mhofman]

I'm not a requested reviewer, nor have I touched this part of swingset before, but I believe this specific change is safe to land.

[FUDCo]

I'm not a requested reviewer, nor have I touched this part of swingset before, but I believe this specific change is safe to land.

Yeah, I think it's fine.

[turadg]

Splitting these tools into their own package has a definite appeal. However, much of what they do is closely coupled to SwingSet internals

#5244 is a new ticket we can use to discuss design and priority.

[kriskowal]

I’d be happy to stamp this if I understood "peerDependencies": how Endo is supposed to treat them (currently ignores), what distinguishes them from "dependencies" and "devDependencies", and how to decide to use them or not. My preference is to elevate "devDependencies" to "dependencies" over consigning them to limbo. What does "peerDependencies" do better than "dependencies" in this case?

[turadg]

What does "peerDependencies" do better than "dependencies" in this case?

https://nodejs.org/es/blog/npm/peer-dependencies/ is a good primer.

In this case, if Ava 3 were in dependencies and the parent project used Ava 4 then Node would happily let tools/ imports use Ava 3 and the importing package use Ava 4 but what we need is for both of them to be on the same version of Ava. peerDependencies says "if the importing package already has Ava then it needs to satisfy this version spec."

[kriskowal]

That’s funny because I was working closely with Domenic on Q when he published that article. I didn’t understand it then or now, but I think you’ve hinted at the difference: With a peerDependency, the failure mode is that yarn would install nothing and log an error if the predicate isn’t met whereas with dependencies the failure mode is that it installs a different version visible only beneath itself in the node_modules tree. Is that right?

Based on your note, I’ve filed endojs/endo#1177 Peer dependencies will be opaque to our bundler until we close that. It sounds like it’s just a matter of merging peerDependencies into dependencies for the purposes of creating a compartment map and that the distinction is otherwise only meaningful to the package manager, as it decides whether to inherit or produce a different child.

[FUDCo]

I think the term "peer dependency" is major part of the confusion, as it's not so much a dependency as a declaration of an invariant. I interpret what it means as "do not comingle me with alien versions of X".

[turadg]

With a peerDependency, the failure mode is that yarn would install nothing and log an error if the predicate isn’t met whereas with dependencies the failure mode is that it installs a different version visible only beneath itself in the node_modules tree. Is that right?

That's my understanding.