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

Typescript and custom serializer support #22

Closed
wookieb opened this issue Sep 1, 2017 · 9 comments
Closed

Typescript and custom serializer support #22

wookieb opened this issue Sep 1, 2017 · 9 comments

Comments

@wookieb
Copy link

wookieb commented Sep 1, 2017

Hi,

Great work with keyv!

Do you mind if I help you rewrite it to typescript?

Also I believe support for custom serializers would be really beneficial to serialize such objects like Maps, Sets or any custom class.
What do you think?

@lukechilds
Copy link
Contributor

lukechilds commented Sep 4, 2017

Thanks for the offer of help with TypeScript, but it's just not something I'm interested in supporting. I don't think it would be that beneficial to a project of this size and I'm really trying to focus on simplicity.

Custom serializers are an interesting idea, although I'm not sure that that's something that belongs in Keyv. I'm already handling all of the standard JSON types (+ Buffers).

It seems like if you wanted to serialise a custom class it would make more sense to implement that in your class rather than create a custom serializer that would only work with Keyv.

e.g:

const foo = new Foo();
const fooObj = foo.toObject();
await keyv.set('foo', fooObj);

// Then in the future
const fooObj = await keyv.get('foo');
const foo = Foo.fromObject(fooObj);

@wookieb
Copy link
Author

wookieb commented Sep 4, 2017

It seems like if you wanted to serialize a custom class it would make more sense to implement that in your class rather than create a custom serializer that would only work with Keyv.

Just having ability to set a function for serialization/deserialization is enough

new Keyv('...', {serializer: myCustomSerializerFunction, deserializer, myCustomerDeserializerFunction});

Now I can fully control how data will be serialized/deserialized.

I don't think it would be that beneficial to a project of this size and I'm really trying to focus on simplicity.

Using typescript for even small libraries have a lot of benefits:

  • best autocompletion ever (right now, IDE is not even able to guess parameters used in constructor for KEYV) which also makes possible to read the docs directly in IDE without looking for README of the project.
  • cleaner and better code with async/await http://wookieb.pl/7-reasons-why-you-should-use-async-await-today/
  • no downsides for Javascript users - huge benefit for Typescript users

@lukechilds
Copy link
Contributor

Just having ability to set a function for serialization/deserialization is enough

I suppose it would be a pretty minimal change. Ok, I'm not totally against this. Still not really convinced it belongs inside Keyv though. Is there a specific use case you have in mind for this?

Using typescript for even small libraries have a lot of benefits

Auto complete is a good point, however I disagree with your other points. Async/await is not a good reason to use TS, it could be achieved with Babel too or Vanilla JS if I dropped support for older Node.js versions.

And I think TS alienates some JS developers from getting involved. It's not a standard. I also really hate the idea of adding a build step. Vanilla JS is better IMO if you have reasonably modern targets.

Could we get the best of both worlds keeping the project vanilla JS but adding TypeScript definitions?

@ghost
Copy link

ghost commented Sep 5, 2017

new Keyv('...', {serializer: myCustomSerializerFunction, deserializer, myCustomerDeserializerFunction});

And when would Key use those two functions? Does it need to recognize when an object is an instance of a certain class and when it isn't?

@lukechilds
Copy link
Contributor

@MySidesTheyAreGone serialize/deserialize would be set to JSONB.stringify/JSONB.parse by default but could be overridden by the user.

@ghost
Copy link

ghost commented Sep 5, 2017

So the function itself would do the class recognition? Ok

@lukechilds
Copy link
Contributor

@wookieb Any interest in contributing TypeScript type definitions?

@wookieb
Copy link
Author

wookieb commented Sep 6, 2017

@lukechilds No I'm not. That introduces typical data synchronization problem where you need to remember to change other file while changing the other. There is no reasonable way to handle that. I understand your decision and I accept it but I'm not going to participate.

@lukechilds
Copy link
Contributor

lukechilds commented Sep 16, 2017

@wookieb I'm gonna close this issue in favour of two new separate issues to make this easier to track.

Thanks for your input!

@mjesun mjesun mentioned this issue Feb 7, 2018
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 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 pushed a commit that referenced this issue Nov 21, 2021
jaredwray added a commit that referenced this issue Nov 21, 2021
* Initial commit

* 0.1.0

* Exclude src from npm and dist from git

* 0.1.1

* Make sure we always transpile for Node.js 4 The AVA preset only targets the Node.js version used to build.

* 0.1.2

* Add usage example to readme

* Mention keyv-redis as existing example

* 1.0.0

* Tweak wording

* `keyv` => Keyv

* Capitalise title

* 1.0.1

* Link to AVA/Keyv

* Fix markdown heading

* 1.0.2

* Update dependencies to enable Greenkeeper 🌴 (#1)

* chore(package): update dependencies

* Readme wording

* Clear store before each test and cleanup afterwards

* Test .clear() deletes all key/value pairs

* Test .clear() resolves to undefiend

* Test .clear() returns a Promise

* Move tests around

* Test .set(key, value) sets a value

* Test .delete(key) deletes a key

* 1.1.0

* Fix lint error

* Add bigger delay to wait for TTL to expire This was sometimes failing on Node.js 4

* 1.1.1

* chore(package): update ava to version 0.21.0 (#2)

* Test value can be an object

* Test value can be a buffer

* Test value can be an object containing a buffer

* Test value can be null

* Test value can be undefined

* Test value can be a number

* .set should now resolve to true

* 1.2.0

* Rename to keyv-test-suite

* Split tests up into seperate files

Allow tests to be imported seperately or use helper to run them all

* Add official tests

* 1.3.0

* Use get-root-module

* Test keyvOfficialTests against keyv-redis

* Run Redis on Travis

* Test namespaced .set(key, value) don't collide

* Rename test to make it clear we're testing both set and get

* Test namespaced delete only deletes from current namespace

* Test namespaced clear only clears current namespace

* Use new store instace for each test

* Fix typo

* 1.4.0

* docs(readme): add Greenkeeper badge (#3)

* Remove Greenkeeper badge

* Use this (renamed get-root-module)

* 1.4.1

* Increase TTL delay to stop false positives

* 1.4.2

* Spoof date if possible

* Test against SQLite not Redis

* Update docs

* Update keywords

* 1.5.0

* Scope to @keyv

* 1.5.1

* Fix timings so tests don't fail for slow writes

* 1.5.2

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

* Test value can contain quotes

* 1.6.0

* Add .npmignore

* 1.6.1

* Remove .npmignore

* 1.6.2

* 1.6.3

* Add .npmignore

* 1.6.4

* Update timekeeper to the latest version 🚀 (#10)

* 1.6.5

* pkg.files over .npmignore

* 1.6.6

* Revert "pkg.files over .npmignore"

This reverts commit 045b652acc6ace1d63bc81aaf6aeeda5577e790f.

* 1.6.7

* Add everything to .npmignore apart from dist

* 1.6.8

* Revert "Add everything to .npmignore apart from dist"

This reverts commit 4495a78b139eb00b062967c43060b7934b8630f0.

* 1.6.9

* Add everything to .npmignore apart from dist

* 1.6.10

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

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

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

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

* await `keyv.clear()` before finishing test (#17)

* 1.6.11

* compliancy -> compliance (#22)

* handling yarn.lock

* updating author

* updating author

* updating author

* adding in build for node 12, 14, and 16

* Delete .travis.yml

* adding in status badge

* adding in the build script

* remving the coverage file

* updating coverage script

* updating badge

* upgrading nyc to version 15.1.0

* upgrading delay to version 2.0.0

* upgrading ava to version 3.15.0

* adding nvm to project for node 16

* upgrading xo to version 0.46.4

* spacing added

* adding in file names

* using require for buffer

* version bump 1.6.12

* adding in dist to ignore

* moving test-suite 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: Roney Rao <roneyrao@hotmail.com>
Co-authored-by: Dan Dascalescu <ddascalescu+github@gmail.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>
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

No branches or pull requests

2 participants