Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nestjs depends on types from devDependency, failing compilation #985

Closed
blumamir opened this issue Apr 28, 2022 · 7 comments · Fixed by #992
Closed

nestjs depends on types from devDependency, failing compilation #985

blumamir opened this issue Apr 28, 2022 · 7 comments · Fixed by #992
Labels
bug Something isn't working

Comments

@blumamir
Copy link
Member

What version of OpenTelemetry are you using?

1.0.1

What version of Node are you using?

v16.14.0

What did you do?

yarn add @opentelemetry/instrumentation-nestjs-core -> "@opentelemetry/instrumentation-nestjs-core": "0.28.3" and then:

import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core';
new NestInstrumentation();

This package does not depend on nestjs directly.

When yarn building the package, following error printed to console:

../../node_modules/@opentelemetry/instrumentation-nestjs-core/build/src/instrumentation.d.ts:2:30 - error TS2307: Cannot find module '@nestjs/core' or its corresponding type declarations.

2 import type * as NestJS from '@nestjs/core';
                               ~~~~~~~~~~~~~~


Found 1 error.

This message is valid, as the nestjs instrumentation indeed does this:

import type * as NestJS from '@nestjs/core';
export class Instrumentation extends InstrumentationBase<typeof NestJS> {

which transpile into the instrumentation.d.ts file in build as:

import type * as NestJS from '@nestjs/core';
export declare class Instrumentation extends InstrumentationBase<typeof NestJS> {

So in order for this package to work correctly in typescript, we will need to include these types in the regular "dependencies" which also means dragging a dependency on the entire @nestjs/core for all users just for the type.

Another alternative would be to remove the type from InstrumentationBase, replacing it with any and thus having no type import in the public interface. It seems to not be used for anything anyhow.

export class Instrumentation extends InstrumentationBase<typeof NestJS> {

// Change to 
export class Instrumentation extends InstrumentationBase<any> {

The other type imports from @nestjs/* are not exported to the public interface so I think they are safe to use in instrumentation.ts:

import type { NestFactory } from '@nestjs/core/nest-factory.js';
import type { RouterExecutionContext } from '@nestjs/core/router/router-execution-context.js';
import type { Controller } from '@nestjs/common/interfaces';

If there is support for this change, I can work on a PR to add it.

@rauno56

@blumamir blumamir added the bug Something isn't working label Apr 28, 2022
@Flarna
Copy link
Member

Flarna commented Apr 29, 2022

We had several issues like this in the past resulting in moving types into dependencies resulting in complains by consumers that the version specified by the instrumentation differs from that one used by the end user.
This is in special a problem for packages which have built in types as instrumentation has to depend on the package itself.

see for example (there are more)
#878
#802

I think we have to get rid of type definitions of the instrumented module in the instrumentation API.

@blumamir
Copy link
Member Author

We had several issues like this in the past resulting in moving types into dependencies resulting in complains by consumers that the version specified by the instrumentation differs from that one used by the end user. This is in special a problem for packages which have built in types as instrumentation has to depend on the package itself.

see for example (there are more) #878 #802

I think we have to get rid of type definitions of the instrumented module in the instrumentation API.

Me too. Would like to start with nestjs as it blocks my current task, and also discuss guidelines or changes to the InstrumentationBase api to address this wider issue.

@rauno56 - You wrote the instrumentation, right? Does it make sense to you to remove the type above?

@Flarna
Copy link
Member

Flarna commented Apr 29, 2022

Some more references:
open-telemetry/opentelemetry-js#2256
open-telemetry/opentelemetry-js#2388

it's not an easy topic and most likely resulting in some breakage. Also the hook concept quite some instrumentation have will be harder to use as we can't use a well known type there without leaking the type dependency.

@blumamir
Copy link
Member Author

Some more references: open-telemetry/opentelemetry-js#2256 open-telemetry/opentelemetry-js#2388

it's not an easy topic and most likely resulting in some breakage. Also the hook concept quite some instrumentation have will be harder to use as we can't use a well known type there without leaking the type dependency.

Thanks for the references, I need to free up some time to dive deep into the details

@rauno56
Copy link
Member

rauno56 commented May 3, 2022

@rauno56 - You wrote the instrumentation, right? Does it make sense to you to remove the type above?

I don't see another solution. For packages that rely on @types/* it's not such a big issue, but others' importing the module itself just isn't a good solution.

This autoinstrumentation PR reveals more:

node_modules/@opentelemetry/instrumentation-aws-sdk/build/src/aws-sdk.d.ts:1:27 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.
1 import type * as AWS from 'aws-sdk';
                            ~~~~~~~~~

node_modules/@opentelemetry/instrumentation-aws-sdk/build/src/types.d.ts:3:27 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.
3 import type * as AWS from 'aws-sdk';
                            ~~~~~~~~~

node_modules/@opentelemetry/instrumentation-cassandra-driver/build/src/instrumentation.d.ts:4:39 - error TS2307: Cannot find module 'cassandra-driver' or its corresponding type declarations.
4 import type * as CassandraDriver from 'cassandra-driver';
                                        ~~~~~~~~~~~~~~~~~~

node_modules/@opentelemetry/instrumentation-knex/build/src/instrumentation.d.ts:3:28 - error TS2307: Cannot find module 'knex' or its corresponding type declarations.
3 import type * as knex from 'knex';
                             ~~~~~~

node_modules/@opentelemetry/instrumentation-mysql2/build/src/instrumentation.d.ts:2:34 - error TS2307: Cannot find module 'mysql2' or its corresponding type declarations.
2 import type * as mysqlTypes from 'mysql2';
                                   ~~~~~~~~

node_modules/@opentelemetry/instrumentation-nestjs-core/build/src/instrumentation.d.ts:2:30 - error TS2307: Cannot find module '@nestjs/core' or its corresponding type declarations.
2 import type * as NestJS from '@nestjs/core';

@ledniy
Copy link
Contributor

ledniy commented May 19, 2022

node_modules/@opentelemetry/instrumentation-aws-sdk/build/src/aws-sdk.d.ts:1:27 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.
1 import type * as AWS from 'aws-sdk';
                            ~~~~~~~~~

node_modules/@opentelemetry/instrumentation-aws-sdk/build/src/types.d.ts:3:27 - error TS2307: Cannot find module 'aws-sdk' or its corresponding type declarations.
3 import type * as AWS from 'aws-sdk';
                            ~~~~~~~~~

node_modules/@opentelemetry/instrumentation-cassandra-driver/build/src/instrumentation.d.ts:4:39 - error TS2307: Cannot find module 'cassandra-driver' or its corresponding type declarations.
4 import type * as CassandraDriver from 'cassandra-driver';
                                        ~~~~~~~~~~~~~~~~~~

node_modules/@opentelemetry/instrumentation-knex/build/src/instrumentation.d.ts:3:28 - error TS2307: Cannot find module 'knex' or its corresponding type declarations.
3 import type * as knex from 'knex';
                             ~~~~~~

node_modules/@opentelemetry/instrumentation-mysql2/build/src/instrumentation.d.ts:2:34 - error TS2307: Cannot find module 'mysql2' or its corresponding type declarations.
2 import type * as mysqlTypes from 'mysql2';
                                   ~~~~~~~~

I'm getting exactly these errors, is there any workaround? I see that there's an upcoming fix #1017 but with the current release cycle idk when to expect it

UPDATE: as a workaround added all of these dependencies to the devDependencies of my project and the build succeeded

@aikoven
Copy link

aikoven commented Jul 20, 2022

A workaround is to add "skipLibCheck": true to compilerOptions in tsconfig.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants