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

Fix TypeScript definitions and add tests to test them #21

Merged
merged 2 commits into from
Nov 20, 2020

Conversation

watson
Copy link
Contributor

@watson watson commented Nov 17, 2020

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
Copy link
Owner

kriszyp commented Nov 17, 2020

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
Copy link
Contributor Author

watson commented Nov 18, 2020

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 kriszyp merged commit 0cd06c0 into kriszyp:master Nov 20, 2020
kriszyp added a commit that referenced this pull request Nov 20, 2020
@kriszyp
Copy link
Owner

kriszyp commented Nov 20, 2020

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
Copy link
Contributor Author

watson commented Nov 23, 2020

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

@watson watson deleted the types branch November 23, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants