iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
582 stars 210 forks source link

Missing dependencies of itwin packages #6018

Open rkkr opened 9 months ago

rkkr commented 9 months ago

Describe the bug "skipLibCheck": true is set in @itwin/build-tools: https://github.com/iTwin/itwinjs-core/blob/ba27495de35b852087506fe305d234362729bd76/tools/build/tsconfig-base.json#L21C8-L21C8

This causes internal builds and external consumers that use @itwin/build-tools to ignore missing dependencies errors.

To Reproduce tsconfig.json:

{
  "compilerOptions": {
    "target": "ES6",
    "module": "CommonJS",
    "outDir": "dist",
    "rootDir": "src"
  }
}

package.json:

{
  "scripts": {
    "build": "tsc"
  },
  "dependencies": {
    "@itwin/core-bentley": "4.1.6",
    "@types/node": "^18.11.9",
    "typescript": "^4.4.0"
  }
}

src\test.ts:

import * as test from "@itwin/core-bentley";

run:

>> node -v
v18.15.0
>> npm -v
9.5.0
>> npm install
>> npm run build
node_modules/@itwin/core-bentley/lib/cjs/Tracing.d.ts:4:71 - error TS2307: Cannot find module '@opentelemetry/api' or its corresponding type declarations.

4 import type { SpanAttributes, SpanContext, SpanOptions, Tracer } from "@opentelemetry/api";
                                                                        ~~~~~~~~~~~~~~~~~~~~

Found 1 error in node_modules/@itwin/core-bentley/lib/cjs/Tracing.d.ts:4

Additional context Similar issues are observed with other packages within @itwin/.

paulius-valiunas commented 9 months ago

I can't speak for all the packages, but @itwin/core-bentley intentionally does not have a dependency on @opentelemetry/api as the user is supposed to provide the implementation. If you decide to use the OpenTelemetry functionality, you will have to add that package to your direct dependencies and the types will be resolved correctly. If you don't use that functionality, then you won't need to know the types anyway. I would keep skipLibCheck enabled because that's just checking code that has already been compiled and tested. Otherwise you will have problems with pretty much any library that has optional peer dependencies.

(not to mention any dependency that uses a @types and you don't have the type definitions installed in your root package)

rkkr commented 9 months ago

And any suggestions how dependencies between @itwin/ could be enforced?

paulius-valiunas commented 9 months ago

could you clarify what exactly you want to enforce? @opentelemetry/api is not a dependency, but if you already have it in your code and are using it, @itwin/core-bentley will use it too. For anything that is an actual dependency that's required for the code to work correctly, there will be an entry in package.json/dependencies

rkkr commented 9 months ago

I'm talking about other packages like https://github.com/iTwin/insights-client/pull/40 where @itwin dependencies are only set in devDependencies.