Dependency Bug - @types/pino breaking typescript in consumers

[RyanThomas73]

Software versions

• OS: Windows
• pact-js-core version(s): Multiple. Testing with 13.12.0
• Node Version: 16.16.0, 18.12.1
• Other Versions:
  • Typescript Versions: 4.7.4, 4.8.4, probably others
  • @types/pino version: 6.3.11 <-- transitively pulled in through pact-core

Issue Checklist

Please confirm the following:

• I have upgraded to the latest
• I have the read the FAQs in the Readme
• (Not Applicable) I have triple checked, that there are no unhandled promises in my code
• (Not Applicable) I have set my log level to debug and attached a log file showing the complete request/response cycle
• For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

Expected behaviour

The typescript compiler works in a project that has the @pact-foundation/pact-core package installed

Actual behaviour

The typescript compiler fails with:
error TS2688: Cannot find type definition file for 'pino-std-serializers'

Steps to reproduce

• Install the @pact-foundation/pact-core package into a project using typescript
• Run the tsc command

Relevant log files

N/A

Additional Details

The problem is in separate typing libraries @types/pino + @types/pino-std-serializers that come through transitively.

NOTE: I've opened a separate issue here with them. It's uncertain at this point if the maintainers plan to address the incorrect package format directly.
pinojs/pino-std-serializers#119

The pact-js-core library is referencing @types/pino with an explicit version of 6.3.11

pact-js-core/package.json

Line 56 in 1d98a9b

"@types/pino": "6.3.11",

This version is problematic because it has wildcard version references:

"@types/pino-pretty": "*"
"@types/pino-std-serializers": "*"

The wildcard @types/pino-std-serializers appears to be pulling in a new 4.0.0 version that is invalid (has no index.d.ts file) and thus breaking typescript functionality. If the maintainers put out an updated 4.0.1 version that is correct the problem will resolve. But if they do not put out a fixed package or if they are slow in doing so, consumers of pact-core will continue to encounter the problem.

An ideal long term solution would be for the pact-js-core library to update to the latest major pino version which ships with it's type definitions built in and eliminate the @types/pino dependency all together.

In the short term I don't think @types/pino needs to be a dependency, I believe it can be changed to a devDependency.

The only reason it would need to be a non dev dependency is if pact-core was re-exporting the pino types as part of the pact-core type definitions which does not appear to be the case.

Looking through the 13.12.0 package contents, for example, there is a pino.d.ts file in the package but it does not appear to be referenced by any of the exported type definitions and thus should be safe to change it to a devDependency` IMO.

[mefellows]

Thanks for the report. I'll see if bumping to the next major version makes a difference and we can go from there.

If you have a moment for a PR, we'd also appreciate that too.

[mefellows]

In the short term I don't think @types/pino needs to be a dependency, I believe it can be changed to a devDependency.
The only reason it would need to be a non dev dependency is if pact-core was re-exporting the pino types as part of the pact-core type definitions which does not appear to be the case.

I don't know if that's strictly true, albeit I don't have a good argument against it right now except to say I've seen issues previously where a dependency (a type) was marked as dev but caused compilation issues of the type you're seeing, despite the types not being re-exported.

I think that's because typescript will compile all types in the tree, so if the types are missing it will barf.

I'll have a go at updating the dependency to the latest now and see if that helps.

[RyanThomas73]

If they're not being re-exported in anyway (i.e. not referenced in any of the .d.ts files) then the typescript compiler should not find them anywhere in the tree to begin with. That should be the case here.

Here is what the pact-core src/logger/index.d.ts file looks like for the installed package. I followed all of the imports and did not find the pino types being included in the tree anywhere, hence why I said they're not being re-exported.

import { LogLevel } from './types';
export declare const DEFAULT_LOG_LEVEL = "info";
export declare const setLogLevel: (level?: LogLevel) => void;
export declare const getLogLevel: () => LogLevel;
export declare const verboseIsImplied: () => boolean;
declare const logFunctions: {
    pactCrash: (message: string, context?: string) => void;
    error: (message: string, context?: string) => void;
    warn: (message: string, context?: string) => void;
    info: (message: string, context?: string) => void;
    debug: (message: string, context?: string) => void;
    trace: (message: string, context?: string) => void;
};
export declare const logErrorAndThrow: (message: string, context?: string | undefined) => never;
export declare const logCrashAndThrow: (message: string, context?: string | undefined) => never;
export default logFunctions;

[TimothyJones]

Thanks so much for doing the leg work on this, @RyanThomas73 !

I followed all of the imports and did not find the pino types being included in the tree anywhere, hence why I said they're not being re-exported.

I'm really pleased to hear this! I thought I fixed this a while ago, but I guess I must have forgotten to move the dependency (Pact used to re-export them, which caused weird compile issues - almost certainly caused by this import you found).

I would have thought that it still wouldn't cause issues since Pact wasn't importing the types, but clearly I am missing something.

[mefellows]

New release has just gone out with the latest pino dep. Could you please check that it resolves the issue?

I created a new project with a default config (e.g. tsc --init) and tsc doesn't fail, but it didn't for the previous version either so I suspect there is another setting/pathway causing the issue.

I won't dig too far given you already have the setup.

[RyanThomas73]

@mefellows
FYI - I was able to test this morning using the new pact-core v13.12.2 package that was released and confirmed that it solved the typing errors.

[mefellows]

Excellent, thanks!