Skip to content

Commit

Permalink
reduce ambient imports and drop maxNodeModuleJsDepth (#9373)
Browse files Browse the repository at this point in the history
refs: #6512

## Description

- further improves recent TypeScript best practices doc
- turns the zoe/exported.js into a .d.ts so it can be used in
triple-slash references
- adopts that and stops the runtime imports of zoe/exported (progress on
#6512)
- remove the `maxNodeModuleJsDepth` now that the types resolve without
- burns down imports so there are no inter-package runtime imports of
typedefs. (leaving just a bit for #6512)

I put commit checkpoints of type coverage to guard against any
regression, though I took them out again when merging


### Security Considerations

<!-- Does this change introduce new assumptions or dependencies that, if
violated, could introduce security vulnerabilities? How does this PR
change the boundaries between mutually-suspicious components? What new
authorities are introduced by this change, perhaps by new API calls?
-->

### Scaling Considerations

<!-- Does this change require or encourage significant increase in
consumption of CPU cycles, RAM, on-chain storage, message exchanges, or
other scarce resources? If so, can that be prevented or mitigated? -->

### Documentation Considerations

<!-- Give our docs folks some hints about what needs to be described to
downstream users.

Backwards compatibility: what happens to existing data or deployments
when this code is shipped? Do we need to instruct users to do something
to upgrade their saved data? If there is no upgrade path possible, how
bad will that be for users?

-->

### Testing Considerations

<!-- Every PR should of course come with tests of its own functionality.
What additional tests are still needed beyond those unit tests? How does
this affect CI, other test automation, or the testnet?
-->

### Upgrade Considerations

<!-- What aspects of this PR are relevant to upgrading live production
systems, and how should they be addressed? -->
  • Loading branch information
mergify[bot] authored May 20, 2024
2 parents b953651 + 8573fad commit 980ff41
Show file tree
Hide file tree
Showing 162 changed files with 94 additions and 340 deletions.
10 changes: 7 additions & 3 deletions docs/typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ Our use of TypeScript has to accomodate both .js development in agoric-sdk (whic
- `.d.ts` for types modules
- package entrypoint(s) exports explicit types
- for packages upon which other packages expect ambient types:
- `exported.js` exports the explicit types and ambient re-exports
- `exported.js` supplies ambients
- don't use runtime imports to get types ([issue](https://github.com/Agoric/agoric-sdk/issues/6512))

## .d.ts modules

We cannot use `.ts` files any modules that are transitively imported into an Endo bundle. The reason is that the Endo bundler doesn't understand `.ts` syntax and we don't want it to until we have sufficient auditability of the transformation. Moreover we've tried to avoid a build step in order to import a module. (The one exception so far is `@agoric/cosmic-proto` because we codegen the types. Those modules are written in `.ts` syntax and build to `.js` by a build step that creates `dist`, which is the package export.)
We cannot use `.ts` files in any modules that are transitively imported into an Endo bundle. The reason is that the Endo bundler doesn't understand `.ts` syntax and we don't want it to until we have sufficient auditability of the transformation. Moreover we've tried to avoid a build step in order to import a module. (The one exception so far is `@agoric/cosmic-proto` because we codegen the types. Those modules are written in `.ts` syntax and build to `.js` by a build step that creates `dist`, which is the package export.)

A `.d.ts` module allows defining the type in `.ts` syntax, without any risk that it will be included in runtime code. The `.js` is what actually gets imported.
### Benefits

- A `.d.ts` module allows defining the type in `.ts` syntax, without any risk that it will be included in runtime code. The `.js` is what actually gets imported.
- Only `.d.ts` files can be used in [triple-slash reference types](https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html#-reference-types-)

The are some consequences to this approach.

Expand Down
3 changes: 1 addition & 2 deletions packages/ERTP/exported.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
/** @see {@link /docs/typescript.md} */
/* eslint-disable -- doesn't understand .d.ts */

// XXX also explicit exports because `@agoric/ertp` top level confuses the type and value of `AssetKind`
export * from './src/types.js';
import '@agoric/notifier/exported.js';

import {
Amount as _Amount,
Expand Down
5 changes: 2 additions & 3 deletions packages/ERTP/src/paymentLedger.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
// @jessie-check

/// <reference types="@agoric/store/exported.js" />

/* eslint-disable no-use-before-define */
import { isPromise } from '@endo/promise-kit';
import { mustMatch, M, keyEQ } from '@agoric/store';
import { AmountMath } from './amountMath.js';
import { preparePaymentKind } from './payment.js';
import { preparePurseKind } from './purse.js';

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/store/exported.js';

import { BrandI, makeIssuerInterfaces } from './typeGuards.js';

/**
Expand Down
4 changes: 1 addition & 3 deletions packages/ERTP/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 1,
},
"compilerOptions": {},
"include": [
// 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)
Expand Down
Empty file removed packages/SwingSet/exported.js
Empty file.
2 changes: 0 additions & 2 deletions packages/SwingSet/src/vats/plugin-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import { HandledPromise } from '@endo/eventual-send';
import { Remotable } from '@endo/marshal';
import { Far, E } from '@endo/far';

import '@agoric/store/exported.js';

/**
* @template T
* @typedef {T | PromiseLike<T>} ERef
Expand Down
2 changes: 0 additions & 2 deletions packages/SwingSet/tools/prepare-test-env-ava.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import '@endo/init/pre-bundle-source.js';

import '@agoric/swingset-liveslots/tools/prepare-test-env.js';

import '@endo/ses-ava/exported.js';

import { wrapTest } from '@endo/ses-ava';
import rawTest from 'ava';

Expand Down
4 changes: 1 addition & 3 deletions packages/SwingSet/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 2,
},
"compilerOptions": {},
"include": [
"exported.js",
"demo/**/*.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/agoric-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,6 @@
"workerThreads": false
},
"typeCoverage": {
"atLeast": 76.88
"atLeast": 76.99
}
}
2 changes: 0 additions & 2 deletions packages/agoric-cli/src/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import chalk from 'chalk';
import { makePspawn } from './helpers.js';

/// <reference types="@endo/captp/src/types.js" />
/// <reference types="@agoric/swingset-vat/exported.js" />
/// <reference types="@agoric/network/exported.js" />

// Use either an absolute template URL, or find it relative to DAPP_URL_BASE.
const gitURL = (relativeOrAbsoluteURL, base) => {
Expand Down
1 change: 0 additions & 1 deletion packages/agoric-cli/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"checkJs": false,
"maxNodeModuleJsDepth": 2,
},
"include": [
"*.js",
Expand Down
1 change: 0 additions & 1 deletion packages/assert/exported.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/assert/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
"homepage": "https://github.com/Agoric/agoric-sdk#readme",
"files": [
"src/",
"exported.js",
"NEWS.md"
],
"publishConfig": {
Expand Down
1 change: 0 additions & 1 deletion packages/assert/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
{
"extends": "../../tsconfig.json",
"include": [
"exported.js",
"src/**/*.js",
"test/**/*.js",
],
Expand Down
2 changes: 1 addition & 1 deletion packages/async-flow/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@
"workerThreads": false
},
"typeCoverage": {
"atLeast": 96.68
"atLeast": 77.83
}
}
3 changes: 1 addition & 2 deletions packages/base-zone/src/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @jessie-check

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/store/exported.js';
/// <reference types="@agoric/store/exported.js" />

// eslint-disable-next-line import/export
export * from './exports.js';
Expand Down
4 changes: 1 addition & 3 deletions packages/base-zone/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 2,
},
"compilerOptions": {},
"include": [
"*.js",
"scripts",
Expand Down
4 changes: 0 additions & 4 deletions packages/benchmark/src/benchmarkerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ import fs from 'node:fs';
import '@endo/init/pre-bundle-source.js';
import '@endo/init';

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/vats/exported.js';
import '@agoric/inter-protocol/exported.js';
import '@agoric/zoe/exported.js';
import '@agoric/cosmic-swingset/src/launch-chain.js';

import { Fail } from '@agoric/assert';
Expand Down
1 change: 0 additions & 1 deletion packages/benchmark/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"compilerOptions": {
"allowImportingTsExtensions": true,
"checkJs": true,
"maxNodeModuleJsDepth": 2,
},
"include": [
"*.js",
Expand Down
3 changes: 0 additions & 3 deletions packages/boot/test/bootstrapTests/zcfProbe.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/zoe/exported.js';

import { makeTracer } from '@agoric/internal';
import { E } from '@endo/far';
import {
Expand Down
3 changes: 0 additions & 3 deletions packages/boot/tools/supports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import { loadSwingsetConfigFile } from '@agoric/swingset-vat';
import { makeSlogSender } from '@agoric/telemetry';
import { TimeMath, Timestamp } from '@agoric/time';

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/vats/exported.js';

import {
boardSlottingMarshaller,
slotToBoardRemote,
Expand Down
1 change: 0 additions & 1 deletion packages/boot/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"compilerOptions": {
"allowImportingTsExtensions": true,
"checkJs": true,
"maxNodeModuleJsDepth": 2,
},
"include": [
"*.js",
Expand Down
1 change: 0 additions & 1 deletion packages/builders/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"checkJs": true,
"maxNodeModuleJsDepth": 2,
},
"include": [
"*.js",
Expand Down
4 changes: 2 additions & 2 deletions packages/cache/src/types.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// @jessie-check

/// <reference types="@agoric/store/exported.js" />

// XXX package factoring debt
import '@agoric/notifier/exported.js';
import '@agoric/store/exported.js';

// Ensure this is a module.
export {};
Expand Down
4 changes: 1 addition & 3 deletions packages/cache/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 1,
},
"compilerOptions": {},
"include": [
"*.js",
"public/**/*.js",
Expand Down
4 changes: 1 addition & 3 deletions packages/casting/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 2,
},
"compilerOptions": {},
"include": [
"*.js",
"public/**/*.js",
Expand Down
1 change: 0 additions & 1 deletion packages/cosmic-swingset/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"checkJs": false,
"maxNodeModuleJsDepth": 1,
},
"include": [
"calc-*.js",
Expand Down
1 change: 0 additions & 1 deletion packages/deploy-script-support/exported.js

This file was deleted.

3 changes: 1 addition & 2 deletions packages/deploy-script-support/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"files": [
"src",
"NEWS.md",
"exported.js"
"NEWS.md"
],
"ava": {
"files": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { makeIssuerKit, AssetKind, AmountMath } from '@agoric/ertp';

import '../../exported.js';

import { makeDepositInvitation } from '../../src/depositInvitation.js';

test('depositInvitation', async t => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { makeIssuerKit, AssetKind, AmountMath } from '@agoric/ertp';

import '../../exported.js';

import { makeOfferAndFindInvitationAmount } from '../../src/offer.js';

test('findInvitationAmount', async t => {
Expand Down
2 changes: 0 additions & 2 deletions packages/deploy-script-support/test/unitTests/install.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { makeFakeBoard } from '@agoric/vats/tools/board-utils.js';
import bundleSource from '@endo/bundle-source';
import { resolve as importMetaResolve } from 'import-meta-resolve';

import '../../exported.js';

import { makeInstall } from '../../src/install.js';

test('install', async t => {
Expand Down
2 changes: 0 additions & 2 deletions packages/deploy-script-support/test/unitTests/offer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import { makeIssuerKit, AmountMath } from '@agoric/ertp';
import { resolve as importMetaResolve } from 'import-meta-resolve';
import { E } from '@endo/far';

import '../../exported.js';

import { makeOfferAndFindInvitationAmount } from '../../src/offer.js';

test('offer', async t => {
Expand Down
4 changes: 1 addition & 3 deletions packages/deploy-script-support/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// This file can contain .js-specific Typescript compiler config.
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 2,
},
"compilerOptions": {},
"include": [
"*.js",
"src/**/*.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/governance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@
"access": "public"
},
"typeCoverage": {
"atLeast": 89.32
"atLeast": 89.31
}
}
3 changes: 1 addition & 2 deletions packages/governance/src/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/// <reference types="@agoric/internal/exported" />
/// <reference types="@agoric/ertp/exported" />
// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/zoe/exported.js';
/// <reference types="@agoric/zoe/exported" />

export {
ChoiceMethod,
Expand Down
1 change: 1 addition & 0 deletions packages/governance/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,7 @@ export {};
* @param {ERef<ZoeService>} zoe
* @param {Instance} allegedGovernor
* @param {Instance} allegedElectorate
* @returns {Promise<true>}
*/

/**
Expand Down
4 changes: 1 addition & 3 deletions packages/governance/test/unitTests/binaryballotCount.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/* eslint @typescript-eslint/no-floating-promises: "warn" */
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/zoe/exported.js';
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { E } from '@endo/eventual-send';
import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
Expand Down
3 changes: 0 additions & 3 deletions packages/governance/test/unitTests/committee.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/zoe/exported.js';

import { makeMockChainStorageRoot } from '@agoric/internal/src/storage-test-utils.js';
import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';
import '@agoric/zoe/exported.js';

import { AmountMath, AssetKind, makeIssuerKit } from '@agoric/ertp';
import { makeRatio } from '@agoric/zoe/src/contractSupport/index.js';

Expand Down
1 change: 0 additions & 1 deletion packages/governance/test/unitTests/paramGovernance.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import '@agoric/zoe/exported.js';
import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { makeMockChainStorageRoot } from '@agoric/internal/src/storage-test-utils.js';
Expand Down
4 changes: 1 addition & 3 deletions packages/governance/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 2,
},
"compilerOptions": {},
"include": [
// 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)
Expand Down
2 changes: 0 additions & 2 deletions packages/inter-protocol/exported.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/inter-protocol/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
"files": [
"scripts",
"src/",
"exported.js",
"NEWS.md"
],
"ava": {
Expand Down
5 changes: 1 addition & 4 deletions packages/inter-protocol/src/auction/auctionBook.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
/// <reference types="@agoric/internal/exported" />
/// <reference types="@agoric/governance/exported" />

// XXX ambient types runtime imports until https://github.com/Agoric/agoric-sdk/issues/6512
import '@agoric/zoe/exported.js';
import '@agoric/zoe/src/contracts/exported.js';
/// <reference types="@agoric/zoe/exported" />

import { AmountMath, RatioShape } from '@agoric/ertp';
import { mustMatch } from '@agoric/store';
Expand Down
Loading

0 comments on commit 980ff41

Please sign in to comment.