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

Add TypeScript type definitions #25

Closed
lukechilds opened this issue Sep 16, 2017 · 8 comments
Closed

Add TypeScript type definitions #25

lukechilds opened this issue Sep 16, 2017 · 8 comments

Comments

@lukechilds
Copy link
Contributor

lukechilds commented Sep 16, 2017

Will enable autocompletion in supported text editors/IDEs.

Previous discussion in #22

To prevent definitions going out of sync we should either include some sort of test to validate the definitions or alternatively they be generated from our source (annotated comments?).

@caseyWebb
Copy link
Contributor

caseyWebb commented Oct 7, 2019

To prevent definitions going out of sync we should either include some sort of test to validate the definitions or alternatively they be generated from our source (annotated comments?).

I saw the previous discussion, but by far the easiest way to do this is rewriting the source in TypeScript and enabling "declarations" in the compiler options. tsd can be used to validate, but doesn't really solve the problem of them going out of sync as the -test-d.ts also has to be updated.

If you've had any change of heart, I've got no problem doing the rewrite and configuring tsc.

@lukechilds
Copy link
Contributor Author

lukechilds commented Oct 8, 2019

Thanks for the offer @caseyWebb ❤️

I kinda figured that was the only maintainable solution.

Tbh I've had a little play with TypeScript and wasn't really sold on it. I didn't hate it, I just didn't really "get it". I'm still open to it.

I'm interested in the idea, but I have some worries:

  1. You port this codebase to TypeScript, I don't like it, don't accept your PR, you feel like you've wasted your time.

  2. You port this codebase to TypeScript, I like it and merge, you disappear, future maintenance becomes much harder (I'm already struggling to find the time) because I'm not as familiar with TypeScript.

There is also the issue that if we re-write this in TypeScript we should probably re-write all storage adapters in TypeScript too.

What are your thoughts?

@caseyWebb
Copy link
Contributor

caseyWebb commented Oct 8, 2019

On the concern of not merging, no harm no foul. This codebase will take 15 minutes, tops.

On the concern of maintainability, it's actually a pretty easy out in worst case scenario. Set the tsconfig.json target to "esnext", transpile, replace the source with the output JS. It will be identical with only the types missing.

I'm more than willing to work on the adapters as well, as while not crucial I do agree that they should all be done. Have you considered/would you be open to a monorepo?

One more thing to note, not being sold on it is a common thread among people. 3 things I like to point out:

  1. your consumers will benefit far more than you will; they won't need to continually hop back and forth with the documentation, and it makes discovery a lot easier.

  2. if you weren't using compilerOptions.strict, you're missing the vast majority of the benefits afforded. Non-strict TS may as well be JS.

  3. it makes eslint way more powerful when used with @typescript-eslint, as it can catch annoying things like unhandled promises via the type information.

@lukechilds
Copy link
Contributor Author

lukechilds commented Oct 8, 2019

You are definitely selling it to me.

Have you considered/would you be open to a monorepo?

I've not been a fan of them historically but this current setup with dependencies spread across repos is definitely not ideal. If you can demonstrate a better solution using a monorepo then I'd be interested.

The maintenance issue is the only thing really worrying me, as I mentioned before I'm already struggling with time as is. Extra build steps and stuff I'm not familiar with, while probably simple to learn, is just extra stuff eating away at time I already don't have a lot of.

How interested are you in this project? Is this a one time contribution you want to make or are you interested in sticking around and becoming a maintainer?

No pressure to commit to anything you don't want to, I just wanna know if I'm gonna be accepting extra maintenance burden by accepting these changes.

@caseyWebb
Copy link
Contributor

If you can demonstrate a better solution using a monorepo then I'd be interested.

Challenge accepted 😉

I totally get the maintainability concerns. Tbh I've almost used keyv in a few projects but the lack of solid TS support led to me rolling my own solutions, so I'm more than willing to maintain it long term. As far as having to learn new stuff, my goal is always to have everything you need exposed via npm run scripts, optionally with docker support if it makes sense; so you, as well as any new contributors, should be able to clone, npm run build, npm test, and expect all green.

I'll do the refactors in a fresh monorepo (with git history maintained) as it makes it a lot easier during active development (esp with regards to docker... it doesn't like npm link), and then it can be decided whether or not to keep them together or cherrypick the changes into the respective packages.

When I've got at least two packages I'll push them so you can get an overview of the proposed architecture and see how you like working in it.

@lukechilds
Copy link
Contributor Author

Thanks @caseyWebb, this sounds really great.

my goal is always to have everything you need exposed via npm run script

Ah, a man after my own heart!

Look forward to seeing what you produce!

@jaredwray
Copy link
Owner

jaredwray commented Oct 9, 2019

It would be better to just define the types: http://blog.wolksoftware.com/contributing-to-definitelytyped

Casey - More than happy to help you with this.

caseyWebb pushed a commit to caseyWebb/keyv that referenced this issue Oct 9, 2019
…y#25)

* Support passing both URL and collection name to constructor

* Add .env support
caseyWebb pushed a commit to caseyWebb/keyv that referenced this issue Oct 9, 2019
…ructor (jaredwray#25)""

And fix version number change

This reverts commit 8f06df9bf96f9201f0cbbbf5825a5778f46da780.
caseyWebb pushed a commit to caseyWebb/keyv that referenced this issue Oct 10, 2019
caseyWebb pushed a commit to caseyWebb/keyv that referenced this issue Oct 10, 2019
caseyWebb pushed a commit to caseyWebb/keyv that referenced this issue Oct 10, 2019
@jaredwray
Copy link
Owner

Going to close this out as the fix should be to move to typescript or add in the definition.

jaredwray added a commit that referenced this issue Nov 21, 2021
* Initial commit

* Fix Travis MySQL

* Mention MariaDB

* Fix invalid bad connection string

* 0.1.0

* Run MySQL setup commands as root

* Use keyv-sql instead of keyv-sequelize (#3)

* Update to requirable

* Pin dependency versions

* 0.2.0

* fix(package): update keyv-sql to version 0.2.4 (#4)

* 0.2.1

* Finish docs

* 1.0.0

* fix(package): update keyv-sql to version 1.0.2 (#6)

Closes #5

* 1.0.1

* Update mysql2 to the latest version 🚀 (#8)

* fix(package): update keyv-sql to version 1.0.3 (#9)

* 1.0.2

* Scope to @keyv

* 1.0.3

* Use scoped sql package

* 1.0.4

* Update ava to the latest version 🚀 (#13)

* Update mysql2 to the latest version 🚀 (#14)

* Update @keyv/sql to the latest version 🚀 (#16)

* fix(package): update @keyv/sql to version 1.0.6

* 1.0.5

* Install typo

* 1.0.6

* fix(package): update mysql2 to version 1.4.2 (#17)

* 1.0.7

* fix(package): update @keyv/sql to version 1.1.0 (#19)

* Update @keyv/sql to the latest version 🚀 (#20)

* Document opts.keySize

* Mention key size issue in readme

* 1.1.0

* Update @keyv/sql to the latest version 🚀 (#21)

* 1.1.1

* Update coveralls to the latest version 🚀 (#22)

* Update ava to the latest version 🚀 (#23)

* Update mysql2 to the latest version 🚀 (#24)

* 1.1.2

* Update mysql2 to the latest version 🚀 (#25)

* 1.1.3

* Update ava to the latest version 🚀 (#26)

* Update ava to the latest version 🚀 (#27)

* Update mysql2 to the latest version 🚀 (#28)

* 1.1.4

* chore(package): update xo to version 0.20.1 (#30)

Closes #29

* Update .gitignore

* upgrading @keyv/sql to version 1.1.3

* Delete .travis.yml

* adding in build for github actions

* removing files for testing mysql

* upgraded mysql2 to version 2.3.1 to get test working

* mysql docker compose file

* updated to use testing user / pass combo

* updating with build status badge

* upgrading @keyv/sql to version 1.1.3

* updating authors and licensing

* upgrading nyc to version 15.1.0

* upgrading ava to version 3.15.0

* updating to handle const instead of import for ava

* Revert "Upgrading ava to version 3.15.0"

* fixing logo on readme

* Update build.yaml

* Update build.yaml

* Update build.yaml

* Update build.yaml

* updating with all mysql values

* moving to root for testing

* updating to handle super user

* moving back to docker

* Update mysql.yaml

* using local container db

* Update build.yaml

* testing mariadb service

* Update build.yaml

* Update build.yaml

* moving to docker compose

* upgrading mysql2 to version 2.3.2

* upgrading xo to version 0.45.0

* updating fixes for xo formating

* upgrading ava to version 3.15.0

* moving to const for testing

* version bump for release

* moving to node 16

* updating to latest compose

* upgrading xo to version 0.46.4

* Update build.yaml

* upgraind mysql2 to version 2.3.3

* adding in codecov

* adding in code coverage

* updating for services

* upgrading keyv to version 4.0.4

* updating coverage reporting

* removing mysql dependency

* removing mysql dependency and fixing clean

* no longer need build in mono

* no longer needed

* importing mysql

Co-authored-by: Luke Childs <lukechilds123@gmail.com>
Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
jaredwray added a commit that referenced this issue Nov 21, 2021
* Initial commit

* docs(readme): add Greenkeeper badge

* Setup entry schema

* Use key as primary key

* Ensure keys are unique and indexable

* Disable timestamps

* Add get/set/delete methods

* Add clear method

* Wait for connection before running queries

* Return Boolean for delete method

* clear method always resolves to undefined

* get method returns undefined on nonexistent keys

* Disable logging by default

* 0.1.0

* Make invalid connection string more clear

* Explicitly use memory for tests

* KeyvSQLite => KeyvSqlite

* Respect table name

* Don't run tests in memory

* Accept connection string as contructor argument

* Make sure clear only clears current namespace

* 0.2.0

* Remove Greenkeeper badge

* Test options have expected defaults

* Extract sequelize logic into subclass

* Remove old test

* Update keywords

* Remove redundant type casting

* 0.3.0

* Logging and table are set by default in keyv-sequelize

* fix(package): update keyv-sequelize to version 0.1.0 (#3)

* Use keyv-sql instead of keyv-sequelize (#5)

* 1.0.0

* Pass busyTimeout option to SQLite

* Use large busyTimeout with tests

Stops 'SQLITE_BUSY: database is locked' errors from creating loads of connections

* fix(package): update keyv-sql to version 0.2.0 (#6)

* Pin dependency versions

* Only accept opts object

* 1.1.0

* fix(package): update keyv-sql to version 0.2.3 (#8)

Closes #7

* Update to requirable

* 1.2.0

* fix(package): update keyv-sql to version 0.2.4 (#9)

* 1.2.1

* Finish docs

* Also mention table option

* fix(package): update keyv-sql to version 1.0.2 (#11)

Closes #10

* 1.2.2

* fix(package): update sqlite3 to version 3.1.9 (#13)

* 1.2.3

* Update keyv-sql to the latest version 🚀 (#14)

* 1.2.4

* Scope to @keyv

* 1.2.5

* Use scoped sql package

* 1.2.6

* Update ava to the latest version 🚀 (#17)

* fix(package): update @keyv/sql to version 1.0.6 (#18)

* fix(package): update @keyv/sql to version 1.1.1 (#22)

Closes #20

* 1.2.7

* Update @keyv/sql to the latest version 🚀 (#23)

* 1.2.8

* Update sqlite3 to the latest version 🚀 (#24)

* 1.2.9

* Update sqlite3 to the latest version 🚀 (#25)

* 1.2.10

* Update sqlite3 to the latest version 🚀 (#26)

* 1.2.11

* Update sqlite3 to the latest version 🚀 (#28)

* 1.2.12

* Update coveralls to the latest version 🚀 (#27)

* Update ava to the latest version 🚀 (#30)

* Update ava to the latest version 🚀 (#31)

* Update ava to the latest version 🚀 (#32)

* fix(package): update sqlite3 to version 4.0.2 (#40)

Closes #34

* Stop testing Node.js v4 and start testing v10

* 2.0.0

* Update sqlite3 to version 4.0.8 (#44)

* Update ava to version 1.4.1 (#46)

* Update nyc to version 14.1.1 (#48)

Closes #36

* 2.0.1

* Update sqlite3 and don't pin version

* 2.0.2

* upgrading travisci to node version 12,14, and 16

* adding github build tests

* updating to just run tests

* updating build name

* removing travisci

* adding github status badge for build to readme

* upgrading pify to version 5.0.0

* upgrading sqlite3 to version 5.0.2

Thanks to @jdesboeufs for this work.

* upgrading ava to 3.15.0

* upgrading nyc to version 15.1.0

* upgrading to xo version 0.45.0

* xo clean up

opts renamed to options
err renamed to error
tabs and spaces

* moving the busy timeout to 3 seconds

* maintenance release and version bump to v.2.0.3

* moving to standard test suite

* adding in nvm for support on 16

* updating licensing to include authors

* Delete yarn.lock

* no longer needed in mono repo

* import into mono repo

* adding in code coverage for sqlite

* removing this as a storage adapter in keyv

Co-authored-by: Luke Childs <lukechilds123@gmail.com>
Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Co-authored-by: Aditya Patadia <adityapatadia@users.noreply.github.com>
jaredwray added a commit that referenced this issue Nov 22, 2021
* Initial commit

* docs(readme): add Greenkeeper badge

* Create Travis DB for tests

* Use pq@6

pq@7 appears to have issues with Sequelize

* 0.1.0

* Keyvpostgres => KeyvPostgres

* Remove Greenkeeper badge

* Use keyv-sql instead of keyv-sequelize

* Format pq query results as array of rows for keyv-sql

* Don't use obj destructuring for Node.js 4 compat

* Remove sequelize dependencies

* Update to keyv-sql 0.2.3 for latest Postgres compat

* Test against Postgres 9.5 on Travis

* Lock dependency versions

* Only accept opts object as constructor argument

* 1.0.0

* requireable => requirable

* 1.0.1

* fix(package): update keyv-sql to version 0.2.4 (#4)

* 1.0.2

* Finish docs

* Update keyv-sql to the latest version 🚀 (#5)

* 1.0.3

* fix(package): update keyv-sql to version 1.0.2 (#6)

Closes #5

* 1.0.4

* Update pg to the latest version 🚀 (#7)

* 1.0.5

* Update keyv-sql to the latest version 🚀 (#8)

* 1.0.6

* Scope to @keyv

* 1.0.7

* Import scoped test suite

* 1.0.8

* Update pg to the latest version 🚀 (#11)

* 1.0.9

* Update pg to the latest version 🚀 (#12)

* 1.0.10

* Update ava to the latest version 🚀 (#13)

* fix(package): update @keyv/sql to version 1.0.6 (#14)

* fix(package): update pg to version 7.2.0 (#15)

* 1.0.11

* Update pg to the latest version 🚀 (#17)

* 1.0.12

* Update @keyv/sql to the latest version 🚀 (#18)

* Update @keyv/sql to the latest version 🚀 (#19)

* 1.0.13

* fix(package): update @keyv/sql to version 1.1.2 (#20)

* 1.0.14

* Update coveralls to the latest version 🚀 (#21)

* Update ava to the latest version 🚀 (#22)

* Update pg to the latest version 🚀 (#23)

* 1.0.15

* Update ava to the latest version 🚀 (#24)

* Update pg to the latest version 🚀 (#25)

* 1.0.16

* Update package.json

Old versions of pg don't work with new versions of node.js (14+)

* moving github actions to build

* moving to local postgres

* updating to create the database

* creating database

* set the user password

* updating command formatting

* updating postgres password

* same user and pass

* user and pass the same

* updating to handle xo upgrade

* updating to handle new format with xo

* upgrading nyc to latest version 15.1.0

* removing travis ci as no longer needed

* updated build status badge on readme

* upgrade xo to version 0.45.0

* fix options to be named correctly

* upgrading ava to version 3.15.0

* moving testing to staged

* fixing tests

* updating to just Official Tests

* Update test.js

* adding in api tests

* adding in value tests

* moving build to 1 parallel process

* update to stop and start the service

* trying createdb

* trying create db

* adding in owner

* adding in multiple stores

* version bump for release v1.0.17

* updated for serial flag on ava

* updated tests to more uniform

* adding in docker compose for testing

* adding in docker compose for build

* adding in docker compose via scripts

* updating to use npm run

* moving to latest version of postgres

* correcting flow of build

* moved docker compose to test folder

* changed naming convention to be more uniform

* updating for new npm run command to start postgres

* updated compose to test folder and removed values

* updated readme around how to test

* naming adjusted

* updating for testing

* Update README.md

* updating licensing for all authors

* adding nvm and also removing yarn to standardize on npm

* upgrading xo to version 0.46.4

* updating build to reference correct testing

* upgrading @keyv/sql to version 1.1.3

* adding codecov on build workflow

* removing codecoverage output from project

* remving coveralls

* updating badge

* updating to run coverage

* version bump to v1.0.18

* no longer needed

* import @keyv/postgres to mono repo

Co-authored-by: Luke Childs <lukechilds123@gmail.com>
Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Co-authored-by: Andrey Frimuchkov <afrimuchkov@yandex.ru>
jaredwray pushed a commit that referenced this issue Dec 2, 2021
* Support passing both URL and collection name to constructor

* Add .env support
jaredwray pushed a commit that referenced this issue Dec 2, 2021
…ructor (#25)""

And fix version number change

This reverts commit 8f06df9bf96f9201f0cbbbf5825a5778f46da780.
jaredwray added a commit that referenced this issue Dec 2, 2021
* Initial commit

* Connect to MongoDB on init

* Use mongojs driver instead of mongodb

* Add set method

* Add get method

* Add missing semi

* Add delete method

* Protect user from accidentally clearing collection

* Add clear method

* Fix lint error

* Make sure nonexistent keys are handled

* Expose options object

* Test Redis URL can be passed in as string

* Test collection option merges into default options

* Test .delete() with no args doesn't empty the collection

* 0.1.0

* Create unique index for keys

* Create TTL index to clear old keys

* 0.2.0

* Ad usage example to readme

* Don't rely on expirey date from Keyv, calculate our own

* Make sure opts.uri from keyv is mapped to opts.url for mongojs

* 0.2.1

* Emit connection errors on keyv

* 0.3.0

* Emit error events

* Update to keyv-api-suite

* Setup requirable

* Add support for clearing namespaced collections

* 0.4.0

* Remove unused dev dep

* Update requireable to ^1.0.1

* Update dependencies to enable Greenkeeper 🌴 (#1)

* chore(package): update dependencies

* docs(readme): add Greenkeeper badge

* reorder

* Remove Greenkeeper badge

* Use "this"

* Update to requirable

* Pin dependency versions

* 1.0.0

* Finish docs

* 1.0.1

* Update mongojs to the latest version 🚀 (#2)

* 1.0.2

* Scope to @keyv

* 1.0.3

* Update ava to the latest version 🚀 (#5)

* Fix readme error

* 1.0.4

* Update coveralls to the latest version 🚀 (#8)

* Update ava to the latest version 🚀 (#9)

* Update ava to the latest version 🚀 (#11)

* Update mongojs to the latest version 🚀 (#13)

* chore(package): update ava to version 0.25.0 (#12)

* chore(package): update xo to version 0.20.1 (#15)

Closes #14

* Drop tests for Node.js 4 and add tests for Node.js 10 (#26)

Was failing anyway, and Node 4 was end-of-life'd in April.

Add instead tests for Node 10 and current.

* Support passing both URL and collection name to constructor (#25)

* Support passing both URL and collection name to constructor

* Add .env support

* Revert "Support passing both URL and collection name to constructor" (#27)

Reverts lukechilds/keyv-mongo#25

* Revert "Revert "Support passing both URL and collection name to constructor (#25)""

And fix version number change

This reverts commit 8f06df9bf96f9201f0cbbbf5825a5778f46da780.

* 1.1.0

* Updating mongojs and pify versions

* Corrected set to use new syntax

* Incremented minor version

* adding support for default options

* Create build.yaml

* updating for the correct format

* updating readme to new build status

* removing travis ci as no longer needed

* thanks @sittingbool for upgrading pify to version 5.0.0

* removing dotenv as it is not needed

* upgrading xo to version 0.45.0

* upgrading nyc to version 15.1.0

* refactoring to a standard way of handling official tests

* updating time and authors

* updating authors

* updating authors in readme

* removing yarn.lock from repository

* updating github location in package

* updating readme to handle logo

* adding in testing docker compose system

* upgrading keyv to version 4.0.4

* upgrading @keyv/test-suite to version 1.6.12

* upgrading xo to version 0.46.4

* upgrading ava to version 3.15.0

* adding in testing coverage on url and uri

* version bump to 1.2.1

* renaming to just mongo

* removing the docker compose

* removing licensing

* updating to remove coveralls and add in correct coverage

* updating to do code coverage for mongo

Co-authored-by: Luke Childs <lukechilds123@gmail.com>
Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Co-authored-by: Dan Dascalescu <ddascalescu+github@gmail.com>
Co-authored-by: Damir Dado Mitrovic <damir@mitrovic.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants