fixes #27 Add TS types #30
Conversation
windupbird144
commented
Nov 1, 2021
windupbird144
commented
- Overload the build function so there is one signature to create a Transform/split2 and one to create a Duplexify
- Added tests with tsd and modified the scripts section of package.json
Add index.d.ts as the definition file, add tests that are run with tsd
index.d.ts
Outdated
| /// <reference types="node" /> | ||
| /// <reference types="duplexify" /> | ||
|
|
||
| import { Duplexify } from "duplexify"; |
There was a problem hiding this comment.
since this is a dependency now for building TS, would be useful to mention installing these types if using ts in readme.
There was a problem hiding this comment.
I made a new commit with instructions to install @types/duplexify and also @types/node for the Stream API
|
|
||
| export { OnUnknown }; | ||
|
|
||
| export default build; |
There was a problem hiding this comment.
why is there no named export for build? there is none in commonjs too?
There was a problem hiding this comment.
Yes, if I'm not mistaken "build" is only a default export in commonjs, not a named export.
const { build } = require('pino-abstract-transport')
console.log(build)
// undefinedAdd instructions to install additional type declartions if using Typescript. Also fixes a few minor typos.
test/types/index.test-d.ts
Outdated
| @@ -0,0 +1,19 @@ | |||
| import build, { OnUnknown } from "../../index"; | |||
| import { expectType } from "tsd"; | |||
| import { Duplexify } from "duplexify"; | |||
There was a problem hiding this comment.
Do not rely on Duplexify types. Using Duplexify is an implementation detail.
There was a problem hiding this comment.
I've removed the dependency on Duplexify and changed the type to Transform.
Pull Request Test Coverage Report for Build 1409810607Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Because duplexify is an implementation detail, change return types of the build function to Transform
| @@ -5,8 +5,8 @@ | |||
| "main": "index.js", | |||
| "scripts": { | |||
| "prepare": "husky install", | |||
| "test": "standard | snazzy && tap test/*.test.js", | |||
| "test-ci": "standard | snazzy && tap test/*.test.js --coverage-report=lcovonly" | |||
| "test": "standard | snazzy && tap test/*.test.js && tsd", | |||
