Conversation
lib/read.d.ts
Outdated
| /// <reference types="node" /> | ||
| import type { ReadLineOptions } from 'node:readline' | ||
|
|
||
| declare function Read<T = string>( |
There was a problem hiding this comment.
It might be safer to just always use string instead of inferring an alternate type from the default option, but I figured this was the least obtrusive way to do it. So if you do const x = await read({ def: 1 }) then x will be string | number. Since the default option isn't forced to be a string in the code, this seems safest, and shouldn't complain if anyone is using it that way today.
|
what's the best way to assert these types remain correct in CI? will |
The @types/read package is not up to date with the async/await interface changes, and it's often convenient to just have these things local in the module anyway.
Well, one easy way is to write the module in TS. I'd be happy to send a PR to do that like the other modules I've been updating, exporting a CJS and MJS version so it all Just Works. But of course, that's a bigger patch. The other way is to write tests in TS, so that any mis-typings cause test failures. The best is to do both, of course. But other than that, no, there's really no reliable way to ensure that the JS and types match, without actually running code in a way that it's forced to compile and check against the actual thing. |
| import { spawn } from 'child_process' | ||
|
|
||
| const esc = (str: string) => | ||
| str.replace(/\(/g, '\\(').replace(/\)/g, '\\)') |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
| import { spawn } from 'child_process' | ||
|
|
||
| const esc = (str: string) => | ||
| str.replace(/\(/g, '\\(').replace(/\)/g, '\\)') |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
|
Updated the PR with two more commits to do those two things. Take both or neither, as you like :) This is the pattern I've been using to ship modules that work in esm or commonjs mode transparently, with types guaranteed to be in sync with the code, etc. It's a mild bummer having to resort to c8 for coverage, since you lose the |
|
I think this is a little more than we're ready to take on at this time. |
Make editors and typescript happy!
The @types/read package is not up to date with the async/await interface changes, and it's often convenient to just have these things local in the module anyway.
References