fix(promise-kit): make strict typing compliant #3397
Conversation
| // deleting p.domain may break functionality. To retain current | ||
| // functionality at the expense of safety, set unsafe to true. | ||
| const unsafe = false; | ||
| if (unsafe) { | ||
| const originalDomain = p.domain; | ||
| const originalDomain = /** @type {*} */ (p).domain; |
There was a problem hiding this comment.
The convention we have in several of the packages is to use any when we really mean any, and reserve * for when we mean "I can't be bothered to type this right now, so I guess I'll punt until somebody cleans up after my mess."
Given that, I'd recommend:
| const originalDomain = /** @type {*} */ (p).domain; | |
| const originalDomain = /** @type {any} */ (p).domain; |
There was a problem hiding this comment.
The convention we have in several of the packages is to use any when we really mean
any, and reserve*for when we mean "I can't be bothered to type this right now, so I guess I'll punt until somebody cleans up after my mess."
Cool. I didn't know that. Is it explained anywhere?
This has nothing whatsoever to do with JavaScript strict-vs-sloppy mode, right? |
None whatsoever. This is purely a type checking matter (https://www.typescriptlang.org/tsconfig#strict). |
| // deleting p.domain may break functionality. To retain current | ||
| // functionality at the expense of safety, set unsafe to true. | ||
| const unsafe = false; | ||
| if (unsafe) { | ||
| const originalDomain = p.domain; | ||
| const originalDomain = /** @type {*} */ (p).domain; |
There was a problem hiding this comment.
The convention we have in several of the packages is to use any when we really mean any, and reserve * for when we mean "I can't be bothered to type this right now, so I guess I'll punt until somebody cleans up after my mess."
Given that, I'd recommend:
| const originalDomain = /** @type {*} */ (p).domain; | |
| const originalDomain = /** @type {any} */ (p).domain; |
mhofman
force-pushed
the
mhofman/fix-promise-kit-internal-types
branch
from
87dbd90 to
3ac3679
Compare
Description
@agoric/promise-kitdoesn't specify atypesentry in itspackage.jsoncausingtscto parse its.jssource to extract jsdoc typing. This parsing would also validate this internal implementation, which would raise errors when the consumer of the package is instrictmode (mostlynoImplicitAny).Security Considerations
There is a single runtime change from
p.domainto'domain' in pwhich is actually more correct.Documentation Considerations
N/A
Testing Considerations
We should enable
"strict": truefor this package'sjsconfig.jsonbut that currently isn't possible becauseglobal.HandledPromiseis only declared in@agoric/eventual-sendwhich is not imported directly bypromise-kit.