typecheck tests and bump typescript-eslint #2674

turadg · 2024-12-30T17:58:02Z

evergreen

Description

The typescript-eslint dep was behind so I tried to bump it. The major version breaking change required linted files to be included in a tsconfig. Files in demo, scripts and test were linted but not typechecked, so I added those to the tsconfigs. Then I did a bunch of work to get them to green.

I tried to commit incrementally and mostly stick to one package per commit. There are no stacked changes so you could review by file in HEAD and not miss anything.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

CI

Compatibility Considerations

Some slight changes to type interfaces but no breaking runtime changes

Upgrade Considerations

Update NEWS.md for user-facing changes.
? necessary?

boneskull · 2025-01-15T01:34:05Z

packages/bundle-source/demo/reexport-fortune-ts.js

maybe exclude this dir from typechecking? same with other frequent @ts-nocheck directives

boneskull · 2025-01-15T01:34:59Z

packages/bundle-source/test/endo-script-format.test.js

@@ -9,6 +9,7 @@ import bundleSource from '../src/index.js';
 * @param {string} entry
 * @param {Options} options
 */
+// @ts-expect-error 'Options' could be instantiated with a different subtype of constraint 'Partial<any>'.
 const generate = async (entry, options = {}) => {


Suggested change

const generate = async (entry, options = {}) => {

const generate = async (entry, options) => {

This should suppress the error, but then you'll need to supply an empty object somewhere below

boneskull · 2025-01-15T01:36:12Z

packages/bundle-source/test/no-transforms.test.js

@@ -11,6 +11,7 @@ test('no-transforms applies no transforms', async t => {
  const entryPath = url.fileURLToPath(
    new URL(`../demo/circular/a.js`, import.meta.url),
  );
+  // @ts-expect-error Property 'endoZipBase64' does not exist on type '{ moduleFormat: "endoScript"; source: string; } | { moduleFormat: "endoZipBase64"; endoZipBase64: string; endoZipBase64Sha512: string; } | { moduleFormat: "nestedEvaluate"; source: string; sourceMap: string; } | { ...; }'.


looks like we'll need to use @overload to define bundleSource

I think we already do, and specifically that passing moduleFormat: 'endoZipBase64' is supposed to obviate this check. The template for bundleSource is supposed to infer which overload is called based on that option and narrow the return type.

boneskull · 2025-01-15T01:38:21Z

packages/common/test/object-meta-map.test.js

@@ -11,6 +11,7 @@ test('test objectMetaMap', async t => {
        ? undefined
        : {
            ...desc,
+            // @ts-expect-error desc.value possibly undefined


I'd probably throw a t.assert.ok(desc.value) before this statement

boneskull

many comments and questions but nothing blockworthy

boneskull · 2025-01-15T01:41:01Z

packages/compartment-mapper/test/_parse-jsonp.js

@@ -19,6 +19,7 @@ export const parseJsonp = (bytes, _specifier, _location, _packageLocation) => {
    const compartment = new Compartment({
      __options__: true,
      globals: harden({
+        // @ts-expect-error


seems like we should enable the rule that requires an explanation for use of @ts-expect-error

boneskull · 2025-01-15T01:41:45Z

packages/compartment-mapper/test/bundle.test.js

@@ -11,6 +11,7 @@ const fixture = new URL(
  import.meta.url,
 ).toString();

+// @ts-expect-error XXX Node interface munging


I wanted this to actually work. 😄

@boneskull investigate why this is causing an error

when I rebased on master it seems to work

boneskull · 2025-01-15T01:43:31Z

packages/compartment-mapper/test/scaffold.js

@@ -88,7 +89,7 @@ export function scaffold(
  assertFixture,
  fixtureAssertionCount,
  {
-    onError,
+    onError = /** @type {(t, {error, title})} */ (undefined),


imo this should be a @ts-expect-error instead of this thing that is plainly untrue 😄

good point. I made it accurate instead

boneskull · 2025-01-15T01:44:01Z

packages/daemon/test/move-hub.js

@@ -37,7 +37,8 @@ export const make = () => {
  };

  /**
-   * @type {NameHub['write']}
+   * @param {string[]} petNamePath


what was wrong with NameHub? do we still need that type?

boneskull · 2025-01-15T01:44:42Z

packages/eslint-plugin/lib/configs/internal.js

    '@typescript-eslint/restrict-plus-operands': 'warn',
+    // XXX override for RESM concession below
+    '@endo/no-optional-chaining': 'off',


wait, I thought this rule was actually needed?

The "RESM concession below" is:

// Agoric still uses Endo dependencies under an emulation of ESM we call RESM // because it is invoked with `node -r esm`. // RESM does not support ?? nor ?. operators, so we must avoid them expressly.

Endo tests are not exported so optional-chaining is fine in tests.

@boneskull we have very recently been liberated from this constraint. Thanks for spotting.

boneskull · 2025-01-15T01:57:06Z

packages/ses/test/property-override.test.js

@@ -122,7 +122,9 @@ test('packages in-the-wild', t => {

  function testContent6() {
    const list = [];
-    list.push = function newPush() {};
+    list.push = function newPush(...args) {
+      return args.length;


boneskull · 2025-01-15T02:00:10Z

packages/ses/test/tame-date.test.js

@@ -16,7 +16,9 @@ test('lockdown start Date is powerful', t => {
 });

 test('lockdown Date.prototype.constructor is powerless', t => {
-  const SharedDate = Date.prototype.constructor;
+  const SharedDate = /** @type {DateConstructor} */ (


Is Date not Date.prototype.constructor? cc @erights

No, SharedDate is attenuated to avoid a timing side-channel by default, as is Date.now() and similarly Math.random() for a deterministic PRNG snooping side-channel. The signature is the same.

boneskull · 2025-01-15T02:00:55Z

packages/ses/tsconfig.json

@@ -2,12 +2,15 @@
  "extends": "../../tsconfig.eslint-base.json",
  "compilerOptions": {
    "allowJs": true,
+    "skipLibCheck": true,


skipLibCheck: false probably also why linting is slow

boneskull · 2025-01-15T02:01:34Z

packages/ses/types.d.ts

@@ -135,10 +135,12 @@ export type ModuleMapHook = (
  moduleSpecifier: string,
 ) => ModuleDescriptor | undefined;
 export type ImportHook = (moduleSpecifier: string) => Promise<ModuleDescriptor>;
-export type ImportNowHook = (moduleSpecifier: string) => ModuleDescriptor;
+export type ImportNowHook = (


Is this true? Can it return undefined?

boneskull · 2025-01-15T02:05:11Z

packages/zip/test/zip.test.js

@@ -19,6 +19,7 @@ test('zip round trip', async t => {

  const reader = new ZipReader(writer.snapshot());
  const text = textDecoder.decode(reader.read('hello/hello.txt'));
+  // @ts-expect-error undefined if file not found


would probably assert the result is truthy instead

kriskowal

This is largely mechanical and I don’t want a big diff like this to stay on the lam for long. Please at least address the runtime behavior regression in RUNME.js with a type exception for now. Filed #2834.

kriskowal · 2025-05-28T19:15:09Z

packages/bundle-source/test/no-transforms.test.js

@@ -11,6 +11,7 @@ test('no-transforms applies no transforms', async t => {
  const entryPath = url.fileURLToPath(
    new URL(`../demo/circular/a.js`, import.meta.url),
  );
+  // @ts-expect-error Property 'endoZipBase64' does not exist on type '{ moduleFormat: "endoScript"; source: string; } | { moduleFormat: "endoZipBase64"; endoZipBase64: string; endoZipBase64Sha512: string; } | { moduleFormat: "nestedEvaluate"; source: string; sourceMap: string; } | { ...; }'.


I think we already do, and specifically that passing moduleFormat: 'endoZipBase64' is supposed to obviate this check. The template for bundleSource is supposed to infer which overload is called based on that option and narrow the return type.

kriskowal · 2025-05-28T19:17:24Z

packages/captp/test/worker.js

 import { Fail } from '@endo/errors';
 import { makeGuest, makeHost } from './traplib.js';

+if (!maybeParentPort) throw new Error('null parentPort');


I guess but do not know that this is safe in the face of port 0 as opposed to undefined. 0 is a special port for listen, but connect won’t ever provide an address with a port of 0 unless there’s an error.

kriskowal · 2025-05-28T19:20:47Z

packages/compartment-mapper/test/policy.test.js

@@ -1,3 +1,4 @@
+// @ts-nocheck


These can no longer be inferred from directory pattern matching? I do like that they’re evident, but also they are numerous.

kriskowal · 2025-05-28T19:22:43Z

packages/env-options/src/env-options.js

+   * @param {string} defaultSetting
+   * @param {string[]} [optOtherValues]
   * If provided, the option value must be included or match `defaultSetting`.
-   * @returns {T}
+   * @returns {string}


I don’t recall why this was a template type before, but there was probably a reason, @erights?

kriskowal · 2025-05-28T19:23:29Z

packages/eslint-plugin/lib/configs/internal.js

    '@typescript-eslint/restrict-plus-operands': 'warn',
+    // XXX override for RESM concession below
+    '@endo/no-optional-chaining': 'off',


@boneskull we have very recently been liberated from this constraint. Thanks for spotting.

kriskowal · 2025-05-28T19:26:38Z

packages/exo/test/non-enumerable-methods.test.js

@@ -42,7 +42,7 @@ test('test defineExoClass', t => {
  t.throws(() => upCounter.incr(-3), {
    message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0',
  });
-  // @ts-expect-error bad arg
+  // FIXME typedef should catch bad arg


Attn @erights, I think we can tolerate a regression in tsc catching a case here. Maybe we can downgrade FIXME to a note that we would not be surprised if ts-expect-error became necessary in a future tsc upgrade.

kriskowal · 2025-05-28T19:28:07Z

packages/import-bundle/demo/source-maps/RUNME.js

-const bundle = await bundleSource(start, {
-  cacheSourceMaps: true,
-});
+const bundle = await bundleSource(start, {});


I think the purposes of this file is to validate that source maps get cached. Let’s keep the code correct at runtime and mark the error as expected, maybe file an issue to fix the signature of bundleSource.

kriskowal · 2025-05-28T19:29:25Z

packages/module-source/test/_native.js

@@ -1,3 +1,4 @@
 /* global globalThis */
 export const NativeModuleSource = globalThis.ModuleSource;
+// @ts-expect-error XXX typedef


Not in this case. I don’t think we have an analogous /// <reference type="@endo/module-source/shim.js"> but perhaps we should.

kriskowal · 2025-05-28T19:30:36Z

packages/ses/test/_harden-mockery.js

@@ -1,8 +1,10 @@
 export const mockHardened = new WeakSet();
+/** @type {import("../types.js").Harden & {isFake?: true}} */


I’d like to converge on this preference opportunistically. It is not a requirement for this change.

kriskowal · 2025-05-28T19:32:40Z

packages/ses/test/tame-date.test.js

@@ -16,7 +16,9 @@ test('lockdown start Date is powerful', t => {
 });

 test('lockdown Date.prototype.constructor is powerless', t => {
-  const SharedDate = Date.prototype.constructor;
+  const SharedDate = /** @type {DateConstructor} */ (


No, SharedDate is attenuated to avoid a timing side-channel by default, as is Date.now() and similarly Math.random() for a deterministic PRNG snooping side-channel. The signature is the same.

hack around, /opt/agoric/endo/packages/bundle-source/demo/reexport-fortune-ts.js 0:0 error Parsing error: /opt/agoric/endo/packages/bundle-source/demo/reexport-fortune-ts.js was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject /opt/agoric/endo/packages/bundle-source/demo/reexport-meaning-js.js 0:0 error Parsing error: /opt/agoric/endo/packages/bundle-source/demo/reexport-meaning-js.js was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject

Work around: dist/types.d.cts:558:16 - error TS2300: Duplicate identifier 'Compartment'. 558 export class Compartment { ~~~~~~~~~~~ types.d.ts:559:16 559 export class Compartment { ~~~~~~~~~~~ 'Compartment' was also declared here.

Without this building exported.js fails because it would overwrite the exported.d.ts in scm

imports packages that aren't strict Found 76 errors in 10 files. Errors Files 6 ../env-options/src/env-options.js:17 2 ../eventual-send/shim.js:4 28 ../eventual-send/src/handled-promise.js:142 8 ../eventual-send/src/local.js:8 3 ../eventual-send/src/message-breakpoints.js:133 4 ../eventual-send/src/postponed.js:18 9 ../eventual-send/src/track-turns.js:20 1 ../init/pre-remoting.js:5 1 ../lockdown/post.js:12 14 ../ses-ava/src/ses-ava-test.js:33

without it the type inference across packages fails in some build steps

turadg requested review from kriskowal and boneskull December 30, 2024 17:58

turadg mentioned this pull request Jan 3, 2025

linting is too slow #2621

Closed

boneskull reviewed Jan 15, 2025

View reviewed changes

boneskull approved these changes Jan 15, 2025

View reviewed changes

turadg force-pushed the ta/lint-bump branch from 8298cdc to 3f5ead9 Compare May 28, 2025 18:07

turadg enabled auto-merge May 28, 2025 18:08

turadg added 4 commits May 28, 2025 12:34

chore: bump yarn to 4.6

b6e6062

chore(deps): bump lint deps

2abec71

chore(deps): bump prettier 3.5.3

c5a5799

chore(deps): major bump typescript-eslint (8.18.2)

15b0fab

kriskowal mentioned this pull request May 28, 2025

Type signature of bundleSource does not capture cacheSourceMaps #2834

Open

kriskowal approved these changes May 28, 2025

View reviewed changes

turadg force-pushed the ta/lint-bump branch from 3f5ead9 to b5afd96 Compare May 28, 2025 19:55

turadg added 13 commits May 28, 2025 13:12

lint(types): include "test"

108ddfe

chore(deps): repair typescript-eslint config

a32d656

chore(types): conform

5523a9f

chore(types): conform 'where"

ddea3d9

chore(types): conform "stream"

5c8fb4e

chore(types): conform "syrup"

d578680

chore(types): conform "ses"

7108ca2

chore(types): conform "pass-style"

0590102

chore(types): conform 'module-source'

22f86fd

chore(types): conform 'netstring'

43970e2

chore(types): conform 'exo'

6f1ab53

chore(types): conform 'far'

066478b

chore(types): conform 'eventual-send'

37d631d

turadg added 23 commits May 28, 2025 13:12

chore(types): conform 'import-bundle'

b8b61fe

chore(types): conform 'nat'

84ffc97

chore(types): conform 'immutable-arraybuffer'

0a74f75

chore(types): conform 'common'

339b31b

chore(types): conform 'init'

20d94ed

chore(types): conform 'zip'

f7a9fcb

chore(types): conform 'daemon'

2464eaf

lint: include eslint-ed files in tsconfig

589c0e5

chore(types): conform

b9c0a35

lint: skipLibCheck in ses

7eb22cf

Work around: dist/types.d.cts:558:16 - error TS2300: Duplicate identifier 'Compartment'. 558 export class Compartment { ~~~~~~~~~~~ types.d.ts:559:16 559 export class Compartment { ~~~~~~~~~~~ 'Compartment' was also declared here.

build: omit d.ts twin from ts build

12324c5

Without this building exported.js fails because it would overwrite the exported.d.ts in scm

test: fix t.not mistkae

bcbebae

fix(types): wrong return getEnvironmentOption

9956173

chore(types): conform panic

3ed8ff4

test: fix ses-ava Ava usage

ae1fae2

chore(types): strictFunctionTypes for compartment-mapper

038fbe4

chore(deps): bump typescript to 5.8.3

6a5a410

chore(types): conform to TS 5.8.3

13c173b

chore(types): remove maxNodeModuleJsDepth

69c75d6

chore(deps): bump typescript-eslint to 8.33

d38899c

fix(types): include cacheSourceMaps in BundleOptions

1a63ace

turadg force-pushed the ta/lint-bump branch from b5afd96 to 74f2037 Compare May 28, 2025 20:23

turadg added 3 commits May 28, 2025 13:41

build: cleanup all TS output

cb3e979

fix(types): provideEndoClient return

a789620

without it the type inference across packages fails in some build steps

fix(types): abort reason optional

a601d86

turadg force-pushed the ta/lint-bump branch from 74f2037 to a601d86 Compare May 28, 2025 20:52

turadg merged commit 26df345 into master May 28, 2025
15 of 16 checks passed

turadg deleted the ta/lint-bump branch May 28, 2025 21:01

	const generate = async (entry, options = {}) => {
	const generate = async (entry, options) => {

		@@ -1,8 +1,10 @@
		export const mockHardened = new WeakSet();
		/** @type {import("../types.js").Harden & {isFake?: true}} */

typecheck tests and bump typescript-eslint #2674

typecheck tests and bump typescript-eslint #2674

Uh oh!

Conversation

turadg commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Compatibility Considerations

Upgrade Considerations

Uh oh!

boneskull Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boneskull Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boneskull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kriskowal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

turadg commented Dec 30, 2024 •

edited

Loading

boneskull Jan 15, 2025 •

edited

Loading

boneskull Jan 15, 2025 •

edited

Loading