kriszyp / lmdb-js

Simple, efficient, ultra-fast, scalable data store wrapper for LMDB
Other
486 stars 39 forks source link

Fix TypeScript definitions and add tests to test them #21

Closed watson closed 3 years ago

watson commented 3 years ago

Using this module in a project that uses @types/node together with Node.js 13 or newer, will generate this error:

./node_modules/lmdb-store/index.d.ts(5,55): error TS2689: Cannot extend an interface 'NodeJS.EventEmitter'. Did you mean 'implements'?

This PR fixes that bug and adds test to ensure that the types are valid.

The types are tested using the tsd tool and you can run them locally using:

npm run test:types

Additionally I took the liberty to run them automatically after npm test so that they will be part of this projects CI - I hope you don't mind 😊

kriszyp commented 3 years ago

Thank you for the types tests. However, I am getting errors when trying to run the types tests:

  test\types\index.test-d.ts:10:0
  ×   2:9   Module "../.." has no exported member open.
  ×   2:15  Module "../.." has no exported member RootDatabase.
  ×  10:0   Parameter type boolean is not identical to argument type any.
  ×  11:0   Parameter type any is not identical to argument type any.
  ×  14:0   Parameter type string | undefined is not identical to argument type any.

It seems like it may be tripped up by the import, and I have tried removing it (and changing to node/events), but then it complains that index.d.ts isn't a module.

I was wondering if you have a higher level tsconfig.json that you are using to configure these modules?

watson commented 3 years ago

Hmm that's odd. No I don't have a tsconfig.json file anywhere in my path. I tried to just clear out all temporary directories to see if TypeScript somehow was running off a local cache, but my local type tests are still passing just fine.

According to the docs for tsd, it should take care of the configuration, so you shouldn't need a tsconfig.json file unless you wanted to overwrite it. By default it uses this config:

{
    "strict": true,
    "jsx": "react",
    "target": "es2017",
    "lib": ["es2017"],
    "module": "commonjs",
    // The following option is set and is not overridable:
    "moduleResolution": "node"
}

Could you possibly try to re-install your node-modules and try again? Maybe you have an old version of typescript installed or something?

kriszyp commented 3 years ago

I changed the declaration block from a module to a namespace and exported it, and that seemed to fix the type checking for me. Does it still work for you @watson ?

watson commented 3 years ago

Tested with v0.9.0 and it looks like it works 👍 Thanks