Merge pull request #9189 from Agoric/ta/fix-zoe-prepack

restore .d.ts emit for several packages

docs/typescript.md

@@ -1,17 +1,12 @@
 # usage of TypeScript
 
-Our use of TypeScript has to accomodate both .js development in agoric-sdk (which cannot import types) and .ts development of consumers of agoric-sdk packages (which can import types). For .js development, we want ambient (global) types so that we don't have to precede each type reference by an import. For .ts development, we want exports from modules so we don't pollute a global namespace.
-
-This means we need two definition files. We could keep those both in SCM, but that would cause more confusion and risk them getting out of sync. We could have one be defined in terms of the other, but since there is no way to reexport to global namespace, it would have to be a secondary definition in the primary. This would typecheck because TS is structurally typed, but the secondary would lose all the documentation on the types.
-
-So our best solution is to have one source of truth for the types and auto-generate for one case from the other. We've chosen to have the ambient types as the source of truth so the SDK development can use them. The SDK consumption can have a build step during packaging, and that's when we make the exported (non-ambient) types.
+Our use of TypeScript has to accomodate both .js development in agoric-sdk (which could not import types until TS 5.5) and .ts development of consumers of agoric-sdk packages (which could always import types). For .js development, we have many ambient (global) types so that we don't have to precede each type reference by an import. For .ts development, we want exports from modules so we don't pollute a global namespace. We are slowly transitioning away from ambient types.
 
 ## Best practices
 
-- `src/types-ambient.js` defines types of the package
-- `src/types.d.ts` is built automatically from types-ambient
-  - `prepack` copies types-ambient.js to types.js and appends 'export {};' to turn it into a module, then builds
-  - `postpack` deletes the new types.js and .d.ts files
+- package entrypoint(s) exports explicit types
+- for packages upon which other packages expect ambient types:
+  - `exported.d.ts` exports the explicit types and ambient re-exports
 
 ## Generating API docs
 

packages/ERTP/exported.d.ts

@@ -1,16 +1,12 @@
 /* eslint-disable -- doesn't understand .d.ts */
-/**
- * @file re-export types into global namespace, for consumers that expect these
- *   to be ambient
- */
 
-// export everything
 export * from './src/types.js';
 
 // XXX re-export types into global namespace, for consumers that expect these to
 //  be ambient. Why the _ prefix? Because without it TS gets confused between the
 //  import and export symbols. h/t https://stackoverflow.com/a/66588974
 //  Note one big downside vs ambients is that these types will appear to be on `globalThis`.
+// UNTIL https://github.com/Agoric/agoric-sdk/issues/6512
 import {
   Amount as _Amount,
   Brand as _Brand,

packages/ERTP/tsconfig.json

@@ -5,8 +5,10 @@
     "maxNodeModuleJsDepth": 1,
   },
   "include": [
-    // omit exported.js because 1) it's empty and would overwrite exported.d.ts
-    // and 2) because it's only for consumption in other packages
+    // omit exported.js because 1) it need not be included in the typecheck of
+    // this package because it's only consumed by other packages and 2)
+    // including it causes the empty exported.js to overwrite the manual
+    // exported.d.ts (which doesn't need any building)
     "src/**/*.js",
     "src/**/*.ts",
     "test"

packages/agoric-cli/src/commands/auction.js

@@ -6,11 +6,9 @@ import { outputActionAndHint } from '../lib/wallet.js';
 
 const { Fail } = assert;
 
-/** @import {ParamTypesMap} from '@agoric/governance/src/contractGovernance/typedParamManager.js' */
-
 /**
- * @template {ParamStateRecord} M
- * @typedef {import('@agoric/governance/src/contractGovernance/typedParamManager.js').ParamTypesMapFromRecord<M>} ParamTypesMapFromRecord
+ * @import {ParamTypesMap, ParamTypesMapFromRecord} from '@agoric/governance/src/contractGovernance/typedParamManager.js'
+ * @import {ParamValueForType} from '@agoric/governance/src/types.js'
  */
 
 /**

packages/agoric-cli/src/commands/gov.js

@@ -13,7 +13,10 @@ import {
   sendAction,
 } from '../lib/wallet.js';
 
-/** @import {OfferSpec} from '@agoric/smart-wallet/src/offers.js' */
+/**
+ * @import {OfferSpec} from '@agoric/smart-wallet/src/offers.js'
+ * @import {QuestionDetails} from '@agoric/governance/src/types.js'
+ */
 
 const collectValues = (val, memo) => {
   memo.push(val);

packages/governance/exported.d.ts

@@ -0,0 +1,37 @@
+/* eslint-disable -- doesn't understand .d.ts */
+
+export * from './src/types.js';
+
+// XXX re-export types into global namespace, for consumers that expect these to
+//  be ambient. Why the _ prefix? Because without it TS gets confused between the
+//  import and export symbols. h/t https://stackoverflow.com/a/66588974
+//  Note one big downside vs ambients is that these types will appear to be on `globalThis`.
+// UNTIL https://github.com/Agoric/agoric-sdk/issues/6512
+import {
+  GovernanceFacetKit as _GovernanceFacetKit,
+  GovernanceTerms as _GovernanceTerms,
+  GovernorCreatorFacet as _GovernorCreatorFacet,
+  GovernanceSubscriptionState as _GovernanceSubscriptionState,
+  GovernorStartedInstallationKit as _GovernorStartedInstallationKit,
+  GovernedCreatorFacet as _GovernedCreatorFacet,
+  ParamStateRecord as _ParamStateRecord,
+  GovernedPublicFacet as _GovernedPublicFacet,
+  CommitteeElectoratePublic as _CommitteeElectoratePublic,
+  GovernedApis as _GovernedApis,
+  GovernableStartFn as _GovernableStartFn,
+} from './src/types.js';
+declare global {
+  export {
+    _CommitteeElectoratePublic as CommitteeElectoratePublic,
+    _GovernableStartFn as GovernableStartFn,
+    _GovernanceFacetKit as GovernanceFacetKit,
+    _GovernanceSubscriptionState as GovernanceSubscriptionState,
+    _GovernanceTerms as GovernanceTerms,
+    _GovernedApis as GovernedApis,
+    _GovernedCreatorFacet as GovernedCreatorFacet,
+    _GovernedPublicFacet as GovernedPublicFacet,
+    _GovernorCreatorFacet as GovernorCreatorFacet,
+    _GovernorStartedInstallationKit as GovernorStartedInstallationKit,
+    _ParamStateRecord as ParamStateRecord,
+  };
+}

packages/governance/exported.js

@@ -1 +1 @@
-import './src/types-ambient.js';
+// Dummy file for .d.ts twin to declare ambients

packages/governance/package.json

@@ -10,8 +10,8 @@
   "scripts": {
     "build": "yarn build:bundles",
     "build:bundles": "node ./scripts/build-bundles.js",
-    "prepack": "echo \"export {}; \" | cat - src/types-ambient.js > src/types.js && tsc --build --clean tsconfig.build.json",
-    "postpack": "git clean -f '*.d.ts*' src/types.js",
+    "prepack": "tsc --build tsconfig.build.json",
+    "postpack": "git clean -f '*.d.ts*'",
     "test": "ava",
     "test:c8": "c8 $C8_OPTIONS ava --config=ava-nesm.config.js",
     "test:xs": "exit 0",
@@ -61,6 +61,7 @@
     "src/",
     "tools/",
     "exported.js",
+    "exported.d.ts",
     "NEWS.md"
   ],
   "ava": {

packages/governance/src/binaryVoteCounter.js

@@ -17,6 +17,10 @@ import {
 } from './typeGuards.js';
 import { makeQuorumCounter } from './quorumCounter.js';
 
+/**
+ * @import {BuildVoteCounter, OutcomeRecord, Position, QuestionSpec, VoteStatistics} from './types.js';
+ */
+
 const { Fail } = assert;
 
 const validateBinaryQuestionSpec = questionSpec => {

packages/governance/src/breakTie.js

@@ -1,3 +1,7 @@
+/**
+ * @import {Position} from './types.js';
+ */
+
 /**
  * Randomly shuffle an array
  * https://stackoverflow.com/a/12646864

packages/governance/src/closingRule.js

@@ -5,6 +5,10 @@
 import { E } from '@endo/eventual-send';
 import { Far } from '@endo/marshal';
 
+/**
+ * @import {CloseVoting} from './types.js';
+ */
+
 /** @type {CloseVoting} */
 export const scheduleClose = (closingRule, closeVoting) => {
   const { timer, deadline } = closingRule;

packages/governance/src/committee.js

@@ -16,6 +16,10 @@ import { QuorumRule } from './question.js';
 import { ElectorateCreatorI, ElectoratePublicI } from './typeGuards.js';
 import { prepareVoterKit } from './voterKit.js';
 
+/**
+ * @import {ElectorateCreatorFacet, CommitteeElectoratePublic, QuestionDetails, OutcomeRecord, AddQuestion} from './types.js';
+ */
+
 const { ceilDivide } = natSafeMath;
 
 /**

packages/governance/src/contractGovernance/governApi.js

@@ -9,6 +9,10 @@ import {
   coerceQuestionSpec,
 } from '../question.js';
 
+/**
+ * @import {Position, ApiGovernor, ApiInvocationIssue, PoserFacet, VoteOnApiInvocation} from '../types.js';
+ */
+
 const { Fail, quote: q } = assert;
 
 /**

packages/governance/src/contractGovernance/governFilter.js

@@ -10,6 +10,10 @@ import {
   coerceQuestionSpec,
 } from '../question.js';
 
+/**
+ * @import {Position, ApiGovernor, ApiInvocationIssue, PoserFacet, VoteOnApiInvocation, FilterGovernor, GovernedCreatorFacet, OfferFilterIssue, VoteOnOfferFilter} from '../types.js';
+ */
+
 /**
  * Make a pair of positions for a question about whether to update the offer
  * filter. If the vote passes, the list of blocked invitation strings will be

packages/governance/src/contractGovernance/governParam.js

@@ -9,6 +9,9 @@ import {
   QuorumRule,
 } from '../question.js';
 import { ParamChangesQuestionDetailsShape } from '../typeGuards.js';
+/**
+ * @import {ParamValue, ParamChangePositions, QuestionSpec, ChangeParamsPosition, ParamChangeIssue, ParamGovernor, ParamManagerRetriever, PoserFacet, VoteOnParamChanges} from '../types.js';
+ */
 
 const { Fail } = assert;
 

packages/governance/src/contractGovernance/paramManager.js

@@ -18,6 +18,10 @@ import {
 } from './assertions.js';
 import { CONTRACT_ELECTORATE } from './governParam.js';
 
+/**
+ * @import {AnyParamManager, GovernanceSubscriptionState, ParamManagerBase, ParamStateRecord, ParamValueTyped, UpdateParams} from '../types.js';
+ */
+
 const { Fail, quote: q } = assert;
 
 /**
@@ -90,7 +94,7 @@ const makeParamManagerBuilder = (publisherKit, zoe) => {
    * @param {Keyword} name
    * @param {unknown} value
    * @param {(val) => void} assertion
-   * @param {ParamType} type
+   * @param {import('../constants.js').ParamType} type
    */
   const buildCopyParam = (name, value, assertion, type) => {
     let current;

packages/governance/src/contractGovernance/typedParamManager.js

@@ -5,6 +5,11 @@ import { makeParamManagerBuilder } from './paramManager.js';
 
 const { Fail, quote: q } = assert;
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, GovernanceSubscriptionState, GovernanceTerms, ParamManagerBase, ParamStateRecord, ParamValueForType, UpdateParams} from '../types.js';
+ * @import {ParamType} from '../constants.js';
+ */
+
 /**
  * @typedef {Record<Keyword, ParamType>} ParamTypesMap
  */

packages/governance/src/contractGovernor.js

@@ -7,6 +7,10 @@ import { CONTRACT_ELECTORATE } from './contractGovernance/governParam.js';
 import { prepareContractGovernorKit } from './contractGovernorKit.js';
 import { ParamChangesQuestionDetailsShape } from './typeGuards.js';
 
+/**
+ * @import {GovernableStartFn, GovernorCreatorFacet, GovernorPublic, ParamChangeIssueDetails} from './types.js';
+ */
+
 const { Fail } = assert;
 
 const trace = makeTracer('CGov', false);

packages/governance/src/contractGovernorKit.js

@@ -16,6 +16,10 @@ import {
 } from './contractGovernance/governParam.js';
 import { ClosingRuleShape, ParamChangesSpecShape } from './typeGuards.js';
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, ClosingRule, GovernableStartFn, LimitedCF, PoserFacet, VoteOnApiInvocation, VoteOnOfferFilter, VoteOnParamChanges} from './types.js';
+ */
+
 const trace = makeTracer('CGK', false);
 
 const ContractGovernorKitI = {

packages/governance/src/contractHelper.js

@@ -10,6 +10,10 @@ import { assertElectorateMatches } from './contractGovernance/paramManager.js';
 import { makeParamManagerFromTerms } from './contractGovernance/typedParamManager.js';
 import { GovernorFacetShape } from './typeGuards.js';
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, GovernanceSubscriptionState, GovernanceTerms, GovernedApis, GovernedCreatorFacet, GovernedPublicFacet} from './types.js';
+ */
+
 const { Fail, quote: q } = assert;
 
 export const GOVERNANCE_STORAGE_KEY = 'governance';

packages/governance/src/electorateTools.js

@@ -2,6 +2,10 @@ import { EmptyProposalShape } from '@agoric/zoe/src/typeGuards.js';
 import { E } from '@endo/eventual-send';
 import { deeplyFulfilled, Far } from '@endo/marshal';
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn} from './types.js';
+ */
+
 /**
  * @typedef {object} QuestionRecord
  * @property {ERef<VoteCounterCreatorFacet>} voteCap

packages/governance/src/index.js

@@ -1,6 +1,8 @@
-/// <reference path="../../network/exported.js" />
-/// <reference path="../../ERTP/exported.js" />
-/// <reference path="../../zoe/exported.js" />
+// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
+import '@agoric/ertp/exported.js';
+import '@agoric/zoe/src/contractFacet/types-ambient.js';
+import '@agoric/zoe/tools/types-ambient.js';
+import '@agoric/zoe/src/types.js';
 
 /// <reference path="./types-ambient.js" />
 

packages/governance/src/multiCandidateVoteCounter.js

@@ -18,6 +18,10 @@ import {
 import { makeQuorumCounter } from './quorumCounter.js';
 import { breakTie } from './breakTie.js';
 
+/**
+ * @import {QuestionSpec, BuildMultiVoteCounter, MultiOutcomeRecord, Position, VoteStatistics} from './types.js';
+ */
+
 const { Fail } = assert;
 
 const validateQuestionSpec = questionSpec => {

packages/governance/src/noActionElectorate.js

@@ -5,6 +5,10 @@ import { EmptyProposalShape } from '@agoric/zoe/src/typeGuards.js';
 
 import { ElectoratePublicI, ElectorateCreatorI } from './typeGuards.js';
 
+/**
+ * @import {ElectoratePublic, ElectorateCreatorFacet} from './types.js';
+ */
+
 /**
  * This Electorate visibly has no members, takes no votes, and approves no
  * changes.

packages/governance/src/question.js

@@ -3,6 +3,10 @@ import { makeHandle } from '@agoric/zoe/src/makeHandle.js';
 
 import { QuestionI, QuestionSpecShape } from './typeGuards.js';
 
+/**
+ * @import {BuildQuestion, PositionIncluded, Question, QuestionSpec} from './types.js';
+ */
+
 // Topics being voted on are 'Questions'. Before a Question is known to a
 // electorate, the parameters can be described with a QuestionSpec. Once the
 // question has been presented to an Electorate, there is a QuestionDetails

packages/governance/src/quorumCounter.js

@@ -2,6 +2,10 @@
 
 import { Far } from '@endo/marshal';
 
+/**
+ * @import {QuorumCounter} from './types.js';
+ */
+
 export const makeQuorumCounter = quorumThreshold => {
   const check = stats => {
     const votes = stats.results.reduce(

packages/governance/src/types-ambient.js → packages/governance/src/types.js

@@ -1,3 +1,5 @@
+export {};
+
 /**
  * @typedef { 'unranked' | 'order' | 'plurality' } ChoiceMethod
  * * UNRANKED: "unranked voting" means that the voter specifies some number of

packages/governance/src/validators.js

@@ -2,6 +2,10 @@ import { E } from '@endo/eventual-send';
 
 const { Fail, quote: q } = assert;
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, AssertContractGovernance, AssertContractElectorate} from './types.js';
+ */
+
 /**
  * Assert that the governed contract was started by the governor. Throws if
  * either direction can't be established. If the call succeeds, then the

packages/governance/src/voterKit.js

@@ -3,6 +3,10 @@ import { defineDurableHandle } from '@agoric/zoe/src/makeHandle.js';
 import { E } from '@endo/eventual-send';
 import { PositionShape, QuestionHandleShape } from './typeGuards.js';
 
+/**
+ * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, CompletedBallet, Position} from './types.js';
+ */
+
 const VoterI = M.interface('voter', {
   castBallotFor: M.call(QuestionHandleShape, M.arrayOf(PositionShape)).returns(
     M.promise(),

packages/governance/test/swingsetTests/committeeBinary/bootstrap.js

@@ -11,6 +11,10 @@ import {
 } from '../../../src/index.js';
 import { remoteNullMarshaller } from '../utils.js';
 
+/**
+ * @import {QuestionDetails} from '../../../src/types.js';
+ */
+
 const { quote: q } = assert;
 
 const makeVoterVat = async (log, vats, zoe) => {

packages/governance/test/swingsetTests/committeeBinary/vat-voter.js

@@ -3,6 +3,10 @@ import { keyEQ } from '@agoric/store';
 import { E } from '@endo/eventual-send';
 import { Far } from '@endo/marshal';
 
+/**
+ * @import {CommitteeElectoratePublic, Issue} from '../../../src/types.js';
+ */
+
 const { quote: q } = assert;
 
 /**