Repository: endojs/endo (js)

Issue:
@endo/far depends on @fast-check/ava via pass-style
I was trying to `endo bundle ./src/txSync.js` where the only import is `@endo/far` and I got:

```
(Error#1)
Error#1: Cannot find dependency ava for file:///home/connolly/projects/finquick/packages/fincaps/node_modules/@fast-check/ava/

  at gatherDependency (packages/compartment-mapper/src/node-modules.js:447:11)
...
```

I asked `yarn why @fast-check/ava` and it says...

```
=> Found "@fast-check/ava@1.1.5"
info Reasons this module exists
   - "@endo#far#@endo#pass-style" depends on it
   - Hoisted from "@endo#far#@endo#pass-style#@fast-check#ava"
```

sure enough, it's not in `devDependencies`:

https://github.com/endojs/endo/blob/9b17577fef74960466d9d7a1d6081e985ce990d0/packages/pass-style/package.json#L34-L36


Pull request:
refactor(pass-style): Make @fast-check/ava a devDependency
Fixes #1741

> [!CAUTION]
> While technically a breaking change, it is pragmatically not marked as such because we are not aware of anything outside of Endo that imports a fast-check arbitrary.

## Description

Rather than importing `fc` directly to export arbitraries, @endo/pass-style/tools.js now returns them from an exported `makeArbitraries` function that requires `fc` as an argument.

Best reviewed [with whitespace ignored](https://github.com/endojs/endo/pull/2645/files?w=1).

### Security Considerations

n/a

### Scaling Considerations

n/a

### Documentation Considerations

n/a

### Testing Considerations

n/a

### Compatibility Considerations

This is a breaking change, but only to @endo/pass-style/tools.js. All imports in this repository have been updated, and I didn't see any in [agoric-sdk](https://github.com/Agoric/agoric-sdk) or [agoric-3-proposals](https://github.com/Agoric/agoric-3-proposals), but if there are any then they will need similar updates.

### Upgrade Considerations

This only affects code intended for testing, and I'm disinclined to even mention it in NEWS.md.

Patch:
diff --git a/packages/pass-style/package.json b/packages/pass-style/package.json
index 612e70ec5..62f849429 100644
--- a/packages/pass-style/package.json
+++ b/packages/pass-style/package.json
@@ -37,12 +37,12 @@
     "@endo/env-options": "workspace:^",
     "@endo/errors": "workspace:^",
     "@endo/eventual-send": "workspace:^",
-    "@endo/promise-kit": "workspace:^",
-    "@fast-check/ava": "^1.1.5"
+    "@endo/promise-kit": "workspace:^"
   },
   "devDependencies": {
     "@endo/init": "workspace:^",
     "@endo/ses-ava": "workspace:^",
+    "@fast-check/ava": "^1.1.5",
     "ava": "^6.1.3",
     "babel-eslint": "^10.1.0",
     "eslint": "^8.57.0",
diff --git a/packages/pass-style/tools.js b/packages/pass-style/tools.js
index 50fd8ca39..0111f431e 100644
--- a/packages/pass-style/tools.js
+++ b/packages/pass-style/tools.js
@@ -2,16 +2,12 @@
 // including testing done by other packages. Code dependent only on this
 // package from production purposes, such as production code in importing
 // packages, should avoid importing tools.
-// Note that locally, the depenencies of tools are still listed as
+// Note that locally, the dependencies of tools are still listed as
 // `dependencies` rather than `devDependencies`.
 
 export {
   exampleAlice,
   exampleBob,
   exampleCarol,
-  arbString,
-  arbKeyLeaf,
-  arbLeaf,
-  arbKey,
-  arbPassable,
+  makeArbitraries,
 } from './tools/arb-passable.js';
diff --git a/packages/pass-style/tools/arb-passable.js b/packages/pass-style/tools/arb-passable.js
index 5c6bcbdfb..202bfcc9e 100644
--- a/packages/pass-style/tools/arb-passable.js
+++ b/packages/pass-style/tools/arb-passable.js
@@ -1,6 +1,5 @@
 // @ts-check
 import '../src/types.js';
-import { fc } from '@fast-check/ava';
 import { Far } from '../src/make-far.js';
 import { makeTagged } from '../src/makeTagged.js';
 
@@ -12,130 +11,144 @@ export const exampleAlice = Far('alice', {});
 export const exampleBob = Far('bob', {});
 export const exampleCarol = Far('carol', {});
 
-export const arbString = fc.oneof(fc.string(), fc.fullUnicodeString());
+/** @param {typeof import('@fast-check/ava').fc} fc */
+export const makeArbitraries = fc => {
+  const arbString = fc.oneof(fc.string(), fc.fullUnicodeString());
 
-const keyableLeaves = [
-  fc.constantFrom(null, undefined, false, true),
-  arbString,
-  arbString.map(s => Symbol.for(s)),
-  // primordial symbols and registered lookalikes
-  fc.constantFrom(
-    ...Object.getOwnPropertyNames(Symbol).flatMap(k => {
-      const v = Symbol[k];
-      if (typeof v !== 'symbol') return [];
-      return [v, Symbol.for(k), Symbol.for(`@@${k}`)];
-    }),
-  ),
-  fc.bigInt(),
-  fc.integer(),
-  fc.constantFrom(-0, NaN, Infinity, -Infinity),
-  fc.record({}),
-  fc.constantFrom(exampleAlice, exampleBob, exampleCarol),
-];
+  const keyableLeaves = [
+    fc.constantFrom(null, undefined, false, true),
+    arbString,
+    arbString.map(s => Symbol.for(s)),
+    // primordial symbols and registered lookalikes
+    fc.constantFrom(
+      ...Object.getOwnPropertyNames(Symbol).flatMap(k => {
+        const v = Symbol[k];
+        if (typeof v !== 'symbol') return [];
+        return [v, Symbol.for(k), Symbol.for(`@@${k}`)];
+      }),
+    ),
+    fc.bigInt(),
+    fc.integer(),
+    fc.constantFrom(-0, NaN, Infinity, -Infinity),
+    fc.record({}),
+    fc.constantFrom(exampleAlice, exampleBob, exampleCarol),
+  ];
 
-export const arbKeyLeaf = fc.oneof(...keyableLeaves);
+  const arbKeyLeaf = fc.oneof(...keyableLeaves);
 
-export const arbLeaf = fc.oneof(
-  ...keyableLeaves,
-  arbString.map(s => Error(s)),
-  // unresolved promise
-  fc.constant(new Promise(() => {})),
-);
+  const arbLeaf = fc.oneof(
+    ...keyableLeaves,
+    arbString.map(s => Error(s)),
+    // unresolved promise
+    fc.constant(new Promise(() => {})),
+  );
 
-const { keyDag } = fc.letrec(tie => {
-  return {
-    keyDag: fc.oneof(
-      { withCrossShrink: true },
-      arbKeyLeaf,
-      fc.array(tie('keyDag')),
-      fc.dictionary(
-        arbString.filter(s => s !== 'then'),
-        tie('keyDag'),
+  const { keyDag } = fc.letrec(tie => {
+    return {
+      keyDag: fc.oneof(
+        { withCrossShrink: true },
+        arbKeyLeaf,
+        fc.array(tie('keyDag')),
+        fc.dictionary(
+          arbString.filter(s => s !== 'then'),
+          tie('keyDag'),
+        ),
       ),
-    ),
-  };
-});
+    };
+  });
 
-const { arbDag } = fc.letrec(tie => {
-  return {
-    arbDag: fc.oneof(
-      { withCrossShrink: true },
-      arbLeaf,
-      fc.array(tie('arbDag')),
-      fc.dictionary(
-        arbString.filter(s => s !== 'then'),
-        tie('arbDag'),
-      ),
-      // A promise for a passable.
-      tie('arbDag').map(v => Promise.resolve(v)),
-      // A tagged value, either of arbitrary type with arbitrary payload
-      // or of known type with arbitrary or explicitly valid payload.
-      // Ordered by increasing complexity.
-      fc
-        .oneof(
-          fc.record({ type: arbString, payload: tie('arbDag') }),
-          fc.record({
-            type: fc.constantFrom('copySet'),
-            payload: fc.oneof(
-              tie('arbDag'),
-              // copySet valid payload is an array of unique passables.
-              // TODO: A valid copySet payload must be a reverse sorted array,
-              // so we should generate some of those as well.
-              fc.uniqueArray(tie('arbDag')),
-            ),
-          }),
-          fc.record({
-            type: fc.constantFrom('copyBag'),
-            payload: fc.oneof(
-              tie('arbDag'),
-              // copyBag valid payload is an array of [passable, count] tuples
-              // in which each passable is unique.
-              // TODO: A valid copyBag payload must be a reverse sorted array,
-              // so we should generate some of those as well.
-              fc.uniqueArray(fc.tuple(tie('arbDag'), fc.bigInt()), {
-                selector: entry => entry[0],
-              }),
-            ),
-          }),
-          fc.record({
-            type: fc.constantFrom('copyMap'),
-            payload: fc.oneof(
-              tie('arbDag'),
-              // copyMap valid payload is a
-              // `{ keys: Passable[], values: Passable[]}`
-              // record in which keys are unique and both arrays have the
-              // same length.
-              // TODO: In a valid copyMap payload, the keys must be a
-              // reverse sorted array, so we should generate some of
-              // those as well.
-              fc
-                .uniqueArray(
-                  fc.record({ key: tie('arbDag'), value: tie('arbDag') }),
-                  { selector: entry => entry.key },
-                )
-                .map(entries => ({
-                  keys: entries.map(({ key }) => key),
-                  values: entries.map(({ value }) => value),
-                })),
-            ),
-          }),
-        )
-        .map(({ type, payload }) =>
-          makeTagged(
-            type,
-            /** @type {import('../src/types.js').Passable} */ (payload),
-          ),
+  const { arbDag } = fc.letrec(tie => {
+    return {
+      arbDag: fc.oneof(
+        { withCrossShrink: true },
+        arbLeaf,
+        fc.array(tie('arbDag')),
+        fc.dictionary(
+          arbString.filter(s => s !== 'then'),
+          tie('arbDag'),
         ),
-    ),
-  };
-});
+        // A promise for a passable.
+        tie('arbDag').map(v => Promise.resolve(v)),
+        // A tagged value, either of arbitrary type with arbitrary payload
+        // or of known type with arbitrary or explicitly valid payload.
+        // Ordered by increasing complexity.
+        fc
+          .oneof(
+            fc.record({ type: arbString, payload: tie('arbDag') }),
+            fc.record({
+              type: fc.constantFrom('copySet'),
+              payload: fc.oneof(
+                tie('arbDag'),
+                // copySet valid payload is an array of unique passables.
+                // TODO: A valid copySet payload must be a reverse sorted array,
+                // so we should generate some of those as well.
+                fc.uniqueArray(tie('arbDag')),
+              ),
+            }),
+            fc.record({
+              type: fc.constantFrom('copyBag'),
+              payload: fc.oneof(
+                tie('arbDag'),
+                // copyBag valid payload is an array of [passable, count] tuples
+                // in which each passable is unique.
+                // TODO: A valid copyBag payload must be a reverse sorted array,
+                // so we should generate some of those as well.
+                fc.uniqueArray(fc.tuple(tie('arbDag'), fc.bigInt()), {
+                  selector: entry => entry[0],
+                }),
+              ),
+            }),
+            fc.record({
+              type: fc.constantFrom('copyMap'),
+              payload: fc.oneof(
+                tie('arbDag'),
+                // copyMap valid payload is a
+                // `{ keys: Passable[], values: Passable[]}`
+                // record in which keys are unique and both arrays have the
+                // same length.
+                // TODO: In a valid copyMap payload, the keys must be a
+                // reverse sorted array, so we should generate some of
+                // those as well.
+                fc
+                  .uniqueArray(
+                    fc.record({ key: tie('arbDag'), value: tie('arbDag') }),
+                    { selector: entry => entry.key },
+                  )
+                  .map(entries => ({
+                    keys: entries.map(({ key }) => key),
+                    values: entries.map(({ value }) => value),
+                  })),
+              ),
+            }),
+          )
+          .map(({ type, payload }) => {
+            const passable = /** @type {import('../src/types.js').Passable} */ (
+              payload
+            );
+            return makeTagged(type, passable);
+          }),
+      ),
+    };
+  });
 
-/**
- * A factory for arbitrary keys.
- */
-export const arbKey = keyDag.map(x => harden(x));
+  /**
+   * A factory for arbitrary keys.
+   */
+  const arbKey = keyDag.map(x => harden(x));
 
-/**
- * A factory for arbitrary passables.
- */
-export const arbPassable = arbDag.map(x => harden(x));
+  /**
+   * A factory for arbitrary passables.
+   */
+  const arbPassable = arbDag.map(x => harden(x));
+
+  return {
+    exampleAlice,
+    exampleBob,
+    exampleCarol,
+    arbString,
+    arbKeyLeaf,
+    arbLeaf,
+    arbKey,
+    arbPassable,
+  };
+};


Test patch:
diff --git a/packages/marshal/test/encodePassable.test.js b/packages/marshal/test/encodePassable.test.js
index dbb8967ca..57f376266 100644
--- a/packages/marshal/test/encodePassable.test.js
+++ b/packages/marshal/test/encodePassable.test.js
@@ -4,7 +4,7 @@ import test from '@endo/ses-ava/prepare-endo.js';
 
 import { fc } from '@fast-check/ava';
 import { Remotable } from '@endo/pass-style';
-import { arbPassable } from '@endo/pass-style/tools.js';
+import { makeArbitraries } from '@endo/pass-style/tools.js';
 import { assert, Fail, q, b } from '@endo/errors';
 
 import {
@@ -15,6 +15,8 @@ import {
 import { compareRank, makeFullOrderComparatorKit } from '../src/rankOrder.js';
 import { unsortedSample } from './_marshal-test-data.js';
 
+const { arbPassable } = makeArbitraries(fc);
+
 const statelessEncodePassableLegacy = makeEncodePassable();
 
 const makeSimplePassableKit = ({ statelessSuffix } = {}) => {
diff --git a/packages/marshal/test/rankOrder.test.js b/packages/marshal/test/rankOrder.test.js
index cbf51bec0..fee305e20 100644
--- a/packages/marshal/test/rankOrder.test.js
+++ b/packages/marshal/test/rankOrder.test.js
@@ -4,7 +4,7 @@ import test from '@endo/ses-ava/prepare-endo.js';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { fc } from '@fast-check/ava';
 import { makeTagged } from '@endo/pass-style';
-import { arbPassable } from '@endo/pass-style/tools.js';
+import { makeArbitraries } from '@endo/pass-style/tools.js';
 
 import { q } from '@endo/errors';
 import {
@@ -18,6 +18,8 @@ import {
 } from '../src/rankOrder.js';
 import { unsortedSample, sortedSample } from './_marshal-test-data.js';
 
+const { arbPassable } = makeArbitraries(fc);
+
 test('compareRank is reflexive', async t => {
   await fc.assert(
     fc.property(arbPassable, x => {
diff --git a/packages/patterns/test/copySet.test.js b/packages/patterns/test/copySet.test.js
index 674588528..6ddd7811e 100644
--- a/packages/patterns/test/copySet.test.js
+++ b/packages/patterns/test/copySet.test.js
@@ -2,12 +2,7 @@ import test from '@endo/ses-ava/prepare-endo.js';
 
 import { fc } from '@fast-check/ava';
 import { makeTagged, getTag, passStyleOf } from '@endo/marshal';
-import {
-  arbKey,
-  exampleAlice,
-  exampleBob,
-  exampleCarol,
-} from '@endo/pass-style/tools.js';
+import { makeArbitraries } from '@endo/pass-style/tools.js';
 import { Fail, q } from '@endo/errors';
 import {
   isCopySet,
@@ -28,6 +23,8 @@ import { M, matches } from '../src/patterns/patternMatchers.js';
 
 import '../src/types.js';
 
+const { arbKey, exampleAlice, exampleBob, exampleCarol } = makeArbitraries(fc);
+
 /** @import { Key } from '../src/types.js'; */
 
 const assertIsCopySet = (t, s) => {