Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce ambient imports and drop maxNodeModuleJsDepth #9373

Merged
merged 26 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0029140
docs: .d.ts benefits
turadg May 17, 2024
2489715
chore(types): rm empty 'exported' module
turadg May 17, 2024
a8f0167
chore: update type-coverage
turadg May 17, 2024
53f8f93
refactor(types): .d.ts for exported
turadg May 17, 2024
c80fca2
chore(types): reduce zoe runtime imports
turadg May 17, 2024
fc01e93
chore(types): reduce zoe runtime imports
turadg May 17, 2024
0c30c37
chore(types): end zoe/exported.js runtime import
turadg May 17, 2024
1289650
lint(types): drop maxNodeModuleJsDepth to 1
turadg May 17, 2024
d3273a5
lint(types): drop maxNodeModuleJsDepth (to zero)
turadg May 17, 2024
5bfe5cc
chore(types): rm unused module
turadg May 18, 2024
f1112c5
chore(types): no runtime imports from /store
turadg May 18, 2024
661042d
chore(types): no runtime imports from /ertp
turadg May 18, 2024
1f02b5c
chore(types): no runtime imports from /notifier
turadg May 18, 2024
a381b6a
chore(types): no runtime imports of /zoe/contracts
turadg May 18, 2024
a44411d
chore(types): no runtime imports of /network
turadg May 18, 2024
cb58a99
chore(types): no runtime imports of ses-ava
turadg May 18, 2024
e47b0aa
chore(types): no runtime imports of /vats
turadg May 18, 2024
4e65c28
chore(types): no runtime imports of /inter-protocol
turadg May 18, 2024
543df85
chore(types): no runtime imports of /internal
turadg May 18, 2024
5bebd20
refactor(types): ERTP exported.js only ambients
turadg May 18, 2024
4e207b9
docs(typescript): where named and ambient exports go
turadg May 18, 2024
089f06d
chore(types): no import of local exported.js
turadg May 18, 2024
3367a51
chore(types): drop noop network/exported.js
turadg May 18, 2024
0930375
chore(types): rm unused import types-ambient
turadg May 18, 2024
f825d5a
BREAKING CHANGE: drop exported.js
turadg May 18, 2024
8573fad
chore: update type-coverage
turadg May 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing that file seems like a breaking change

for example, consider a dapp that imports it.

comment belongs on the deleted file, but I don't see how to do that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true. in practice we know all the code that relies on it:
https://github.com/search?q=org%3AAgoric+swingset-vat%2Fexported&type=code

https://github.com/Agoric/dapp-pegasus/blob/f3b8f0400e9921a7497b08385d2b3aa609829182/api/deploy.js#L9

that looks ok to me to drop but if you want a BREAKING CHANGE commit I'm game. the package hasn't hit version 1 yet so it won't be a major version bump

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to accommodate 3rd party developers, yes, I think a BREAKING CHANGE commit is worthwhile.

For example:

https://github.com/Kryha/KREAd/blob/c5c5d9e4b7cc370b0adfadc407ee7199c2064c98/agoric/contract/src/kreadKit.js#L43

Copy link
Member Author

@turadg turadg May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the exported.js imported in that repo have been removed https://github.com/search?q=repo%3AKryha%2FKREAd%20exported&type=code

But a few have and I it'll be faster to do this than convince you otherwise ;-) I'll amend the commit message before I merge. I do wish we didn't have to modify commit history in order to note such things

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. I misread this PR as deleting @agoric/zoe/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
Loading