Skip to content

Commit

Permalink
fix(promise-kit): make strict typing compliant (#3397)
Browse files Browse the repository at this point in the history
## Description

`@agoric/promise-kit` doesn't specify a `types` entry in its `package.json` causing `tsc` to parse its `.js` source to extract jsdoc typing. This parsing would also validate this internal implementation, which would raise errors when the consumer of the package is in `strict` mode (mostly `noImplicitAny`).

### Security Considerations

There is a single runtime change from `p.domain` to `'domain' in p` which is actually more correct.

### Documentation Considerations

N/A

### Testing Considerations

We should enable `"strict": true` for this package's `jsconfig.json` but that currently isn't possible because `global.HandledPromise` is only declared in `@agoric/eventual-send` which is not imported directly by `promise-kit`.
  • Loading branch information
mhofman authored Jul 24, 2021
1 parent 5704f80 commit 69e2692
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions packages/promise-kit/src/promiseKit.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// eslint-disable-next-line spaced-comment
/// <reference types="ses"/>

/** @type {import('@agoric/eventual-send').HandledPromiseConstructor | PromiseConstructor} */
const BestPipelinablePromise = globalThis.HandledPromise || Promise;

/**
Expand All @@ -26,7 +27,7 @@ const BestPipelinablePromise = globalThis.HandledPromise || Promise;
/**
* Needed to prevent type errors where functions are detected to be undefined.
*/
const NOOP_INITIALIZER = harden(_ => {});
const NOOP_INITIALIZER = harden(() => {});

/**
* makePromiseKit() builds a Promise object, and returns a record
Expand All @@ -37,19 +38,20 @@ const NOOP_INITIALIZER = harden(_ => {});
* @returns {PromiseRecord<T>}
*/
export function makePromiseKit() {
/** @type {(value: T) => void} */
/** @type {(value: ERef<T>) => void} */
let res = NOOP_INITIALIZER;
/** @type {(reason: any) => void} */
let rej = NOOP_INITIALIZER;

/** @type {Promise<any> & {domain?: unknown}} */
const p = new BestPipelinablePromise((resolve, reject) => {
res = resolve;
rej = reject;
});
// Node.js adds the `domain` property which is not a standard
// property on Promise. Because we do not know it to be ocap-safe,
// we remove it.
if (p.domain) {
if ('domain' in p) {
// deleting p.domain may break functionality. To retain current
// functionality at the expense of safety, set unsafe to true.
const unsafe = false;
Expand Down

0 comments on commit 69e2692

Please sign in to comment.