grafana / pyroscope-nodejs

Pyroscope NodeJS integration
Apache License 2.0
27 stars 21 forks source link

Typescript types are not properly emitted #37

Closed notaphplover closed 7 months ago

notaphplover commented 10 months ago

Description

I cannot easily access types from the package in order to use it in my ts codebase.

Reproduction of the issue

  1. Install any NodeJS >= 16.
  2. Given a root directory, initialize a new npm package: npm init
  3. Install typescript@5 and @pyroscope/nodejs@0.2.6 dependencies.
  4. Add a tsconfig. An example might be:
{
  "$schema": "http://json.schemastore.org/tsconfig",
  "compilerOptions": {
    "declaration": true,
    "declarationMap": true,
    "experimentalDecorators": true,
    "esModuleInterop": true,
    "exactOptionalPropertyTypes": true,
    "forceConsistentCasingInFileNames": true,
    "lib": ["ES2022"],
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "noFallthroughCasesInSwitch": true,
    "noImplicitOverride": true,
    "noImplicitReturns": true,
    "noPropertyAccessFromIndexSignature": true,
    "noUncheckedIndexedAccess": true,
    "resolveJsonModule": true,
    "skipLibCheck": true,
    "sourceMap": true,
    "strict": true,
    "target": "ES2022",
    "outDir": "./dist",
    "rootDir": "./src"
  },
  "include": ["src"]
}
  1. Add a src/index.ts source file:
import Pyroscope from '@pyroscope/nodejs';

Pyroscope.init({
  appName: 'nodejs',
  serverAddress: 'http://localhost:4040',
  sourceMapPath: ['.'],
});
Pyroscope.startHeapProfiling();
Pyroscope.startCpuProfiling();
  1. Compile source files.

Expected behavior

Actual behavior:

error TS7016: Could not find a declaration file for module '@pyroscope/nodejs'. '/home/bob/[...]/node_modules/@pyroscope/nodejs/dist/cjs/index.js' implicitly has an 'any' type.

Additional context.

Consider this tool when solving this issue, it allows you to upload the artifact generated by the npm pack script in order to analyze it.

voxlol commented 9 months ago

I run into this same exact error as well. It's a bummer for my company because the profiling feature in Grafana looks great, but until this is fixed, my company cannot integration with pyroscope for our NodeJS typescript applications

kolesnikovae commented 7 months ago

Thank you @notaphplover for the well described issue and sorry for the delay. Please see https://github.com/grafana/pyroscope-nodejs/pull/42

notaphplover commented 7 months ago

You're very welcome @kolesnikovae , have you tested the artifact? I would hazard to say the package.json should be updated removing the types property. Otherwise types won't be correctly applied

Edit: It seems it's working :smile:

kolesnikovae commented 7 months ago

Thank you @notaphplover, I'm glad to hear this. I've tested the artifact with the tool you suggested (super handy!) and also in a simple test program. I think you're right – the "types" directive is not used anymore – I'll take care of it