Add TypeScript typings #63

evgenymarkov · 2020-08-22T16:23:22Z

Hello! I tried to use this plugin in my project written in TypeScript and it didn't work due to lack of typings. Please check my changes. I can polish them if there are any comments.

Caveats:

Transact helper seems overcomplicated. I couldn't write the types for this case:

fastify.pg.transact((client, commit) => {
  client.query('INSERT INTO users(username) VALUES($1) RETURNING id', [req.params.username], (err, id) => {
    commit(err, id)
  });
});

Also, this is very dangerous code, because if you forget to add catch() to the transaction, you will get an unhandled promise rejection error.

Checklist

run npm run test and npm run benchmark
tests and/or benchmarks are included
documentation is changed or added
commit message and code follows the Developer's Certification of Origin
and the Code of conduct

Videos from VSCode: database, transaction.

kibertoad · 2020-08-22T16:25:36Z

Thanks! Can you also add tsd tests?

evgenymarkov · 2020-08-22T16:33:06Z

@kibertoad I tried to write tsd tests, but tsd crashes with named tuples introduced in typescript 4.0. Named tuples are used to describe the Node-style callback function.

Do you know any other ways to describe callbacks like this?

cb(error: null, result: SomeResult);
cb(error: SomeError, result: undefined);

kibertoad · 2020-08-22T16:45:04Z

@evgenymarkov Isn't named tuple an overkill here? Why not just do

type CallbackXYZ = (error?: SomeError, result?: SomeResult) => void;

kibertoad · 2020-08-22T16:48:56Z

examples/typescript/multiple-db/tsconfig.json

+  "compilerOptions": {
+    /* Basic Options */
+    "noEmit": true,
+    "lib": ["ES2020"],


why this version specifically?

It just example. I downgraded target settings in 4851fbd.

kibertoad · 2020-08-22T16:50:43Z

index.d.ts

+  transact: typeof transact;
+};
+
+type Options = {


Why aren't Options exported?

Fixed in f2a868e.

kibertoad · 2020-08-22T16:51:55Z

tools/build-examples.sh

+
+set -ex
+
+npx tsc --project ./examples/typescript/single-db/tsconfig.json


this is not cross-platform. why not make it an npm script instead?

Fixed in 9e1ca07

kibertoad · 2020-08-22T16:52:41Z

tsconfig.json

@@ -0,0 +1,14 @@
+{
+  "include": ["./index.d.ts"],
+


I would suggest following fastify config: https://github.com/fastify/fastify/blob/master/types/tsconfig.json

evgenymarkov · 2020-08-22T17:06:27Z

@kibertoad Thank you for review! I will respond or fix the comments asap.

kibertoad

LGTM

mcollina

lgtm

evgenymarkov · 2020-08-23T14:11:01Z

@darkgl0w Hi, do you want to review pull request?

examples/typescript/multiple-db/tsconfig.json

examples/typescript/single-db/tsconfig.json

examples/typescript/multiple-db/tsconfig.json

examples/typescript/single-db/tsconfig.json

index.d.ts

package.json

types/tsconfig.json

fox1t

LGTM. Great work!

BlackGlory · 2020-09-11T05:55:35Z

It should be released as v3.0.2, the current version does not have index.d.ts.

* Types: Add "pg" property to FastifyInstance As recommended in the guide for creating TypeScript compatible Fastify plugins, properties that the Fastify instances are decorated with should also in the types get added to those instances: https://www.fastify.io/docs/latest/TypeScript/#creating-a-typescript-fastify-plugin This change follows up on #63 by adding that. * Document "name" option * Enable use of non-docker postgres db in tests * Fix tests when no error is thrown * Throw if named and unnamed are combined * Clarify that intention is to manually add type

feat: add typescript typings

a1a1c69

kibertoad reviewed Aug 22, 2020

View reviewed changes

Evgeny Markov added 7 commits August 22, 2020 20:12

fix: remove unnecessary named tuple

0da50e2

fix: export plugin options

f2a868e

fix: sync typescript configuration with fastify

a2ed13f

fix: make type checking faster and more portable

9e1ca07

fix: downgrade typescript target to node 10 in examples

4851fbd

test: add tests for typings

10eb8d5

chore: sync tsconfig placement with fastify

f13afb8

evgenymarkov requested a review from kibertoad August 23, 2020 00:42

kibertoad approved these changes Aug 23, 2020

View reviewed changes

kibertoad requested review from delvedor, kibertoad, mcollina and Ethan-Arrowood August 23, 2020 10:47

mcollina approved these changes Aug 23, 2020

View reviewed changes

fox1t requested changes Aug 23, 2020

View reviewed changes

Evgeny Markov added 5 commits August 29, 2020 14:52

fix: use base tsconfig files for examples

9bbd477

fix: add named export for PostgresPlugin

b3f2c37

fix: use types from pg namespace

97deecf

fix: remove pg typings from peerDeps

4c9b694

fix: move examples typechecking to separated script

db10fe8

evgenymarkov requested a review from fox1t August 29, 2020 21:43

fox1t approved these changes Sep 2, 2020

View reviewed changes

kibertoad approved these changes Sep 2, 2020

View reviewed changes

kibertoad merged commit b98bee8 into fastify:master Sep 2, 2020

evgenymarkov deleted the typings branch September 2, 2020 23:19

voxpelli mentioned this pull request Nov 9, 2020

Types: Add "pg" property to FastifyInstance #68

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TypeScript typings #63

Add TypeScript typings #63

evgenymarkov commented Aug 22, 2020 •

edited

Loading

kibertoad commented Aug 22, 2020

evgenymarkov commented Aug 22, 2020

kibertoad commented Aug 22, 2020

kibertoad Aug 22, 2020

evgenymarkov Aug 22, 2020

kibertoad Aug 22, 2020

evgenymarkov Aug 22, 2020 •

edited

Loading

kibertoad Aug 22, 2020

evgenymarkov Aug 22, 2020

kibertoad Aug 22, 2020

evgenymarkov Aug 22, 2020

evgenymarkov commented Aug 22, 2020

kibertoad left a comment

mcollina left a comment

evgenymarkov commented Aug 23, 2020

fox1t left a comment

BlackGlory commented Sep 11, 2020


		set -ex

		npx tsc --project ./examples/typescript/single-db/tsconfig.json

Add TypeScript typings #63

Add TypeScript typings #63

Conversation

evgenymarkov commented Aug 22, 2020 • edited Loading

Checklist

kibertoad commented Aug 22, 2020

evgenymarkov commented Aug 22, 2020

kibertoad commented Aug 22, 2020

kibertoad Aug 22, 2020

Choose a reason for hiding this comment

evgenymarkov Aug 22, 2020

Choose a reason for hiding this comment

kibertoad Aug 22, 2020

Choose a reason for hiding this comment

evgenymarkov Aug 22, 2020 • edited Loading

Choose a reason for hiding this comment

kibertoad Aug 22, 2020

Choose a reason for hiding this comment

evgenymarkov Aug 22, 2020

Choose a reason for hiding this comment

kibertoad Aug 22, 2020

Choose a reason for hiding this comment

evgenymarkov Aug 22, 2020

Choose a reason for hiding this comment

evgenymarkov commented Aug 22, 2020

kibertoad left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

evgenymarkov commented Aug 23, 2020

fox1t left a comment

Choose a reason for hiding this comment

BlackGlory commented Sep 11, 2020

evgenymarkov commented Aug 22, 2020 •

edited

Loading

evgenymarkov Aug 22, 2020 •

edited

Loading