Merge pull request #223 from DestinyItemManager/upgrade-types

Upgrade stuff + get rid of any + lints

.github/workflows/deploy.yml

@@ -30,7 +30,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v3
         with:
-          node-version: 18.x
+          node-version-file: '.nvmrc'
           cache: pnpm
 
       - name: pnpm install

.github/workflows/pr-build.yml

@@ -35,7 +35,7 @@ jobs:
       - name: Setup Node
         uses: actions/setup-node@v3
         with:
-          node-version: 18.x
+          node-version-file: '.nvmrc'
           cache: pnpm
 
       - name: Install

.github/workflows/publish.yml

@@ -30,7 +30,7 @@ jobs:
         if: steps.check.outputs.changed == 'true'
         uses: actions/setup-node@v3
         with:
-          node-version: 18.x
+          node-version-file: '.nvmrc'
           registry-url: https://registry.npmjs.org/
 
       - name: Install

.gitignore

@@ -13,7 +13,6 @@ ca.key
 .brackets.json
 .npmrc
 npm-debug.log
-.nvmrc
 yarn-error.log
 coverage/
 dim-api-types/*.js

.nvmrc

@@ -0,0 +1 @@
+20

api/db/apps-queries.test.ts

@@ -1,3 +1,4 @@
+import { DatabaseError } from 'pg';
 import { v4 as uuid } from 'uuid';
 import { ApiApp } from '../shapes/app.js';
 import { getAllApps, getAppById, insertApp } from './apps-queries.js';
@@ -32,7 +33,10 @@ it('cannot create a new app with the same name as an existing one', async () =>
     try {
       await insertApp(client, app);
     } catch (e) {
-      expect(e.code).toBe('23505');
+      if (!(e instanceof DatabaseError)) {
+        fail('should have thrown a DatabaseError');
+      }
+      expect((e).code).toBe('23505');
     }
   });
 });

api/db/apps-queries.ts

@@ -1,54 +1,42 @@
 import { ClientBase, QueryResult } from 'pg';
 import { ApiApp } from '../shapes/app.js';
-import { camelize } from '../utils.js';
+import { camelize, KeysToSnakeCase, TypesForKeys } from '../utils.js';
 
 /**
  * Get all registered apps.
  */
 export async function getAllApps(client: ClientBase): Promise<ApiApp[]> {
-  try {
-    const results = await client.query<ApiApp>({
-      name: 'get_all_apps',
-      text: 'SELECT * FROM apps',
-    });
-    return results.rows.map((row) => camelize<ApiApp>(row));
-  } catch (e) {
-    throw new Error(e.name + ': ' + e.message);
-  }
+  const results = await client.query<KeysToSnakeCase<ApiApp>>({
+    name: 'get_all_apps',
+    text: 'SELECT * FROM apps',
+  });
+  return results.rows.map((row) => camelize(row));
 }
 
 /**
  * Get an app by its ID.
  */
 export async function getAppById(client: ClientBase, id: string): Promise<ApiApp | null> {
-  try {
-    const results = await client.query<ApiApp>({
-      name: 'get_apps',
-      text: 'SELECT * FROM apps where id = $1',
-      values: [id],
-    });
-    if (results.rows.length > 0) {
-      return camelize<ApiApp>(results.rows[0]);
-    } else {
-      return null;
-    }
-  } catch (e) {
-    throw new Error(e.name + ': ' + e.message);
+  const results = await client.query<KeysToSnakeCase<ApiApp>>({
+    name: 'get_apps',
+    text: 'SELECT * FROM apps where id = $1',
+    values: [id],
+  });
+  if (results.rows.length > 0) {
+    return camelize(results.rows[0]);
+  } else {
+    return null;
   }
 }
 
 /**
  * Insert a new app into the list of registered apps.
  */
-export async function insertApp(client: ClientBase, app: ApiApp): Promise<QueryResult<any>> {
-  try {
-    return client.query({
-      name: 'insert_app',
-      text: `insert into apps (id, bungie_api_key, dim_api_key, origin)
+export async function insertApp(client: ClientBase, app: ApiApp): Promise<QueryResult> {
+  return client.query<any, TypesForKeys<ApiApp, ['id', 'bungieApiKey', 'dimApiKey', 'origin']>>({
+    name: 'insert_app',
+    text: `insert into apps (id, bungie_api_key, dim_api_key, origin)
 values ($1, $2, $3, $4)`,
-      values: [app.id, app.bungieApiKey, app.dimApiKey, app.origin],
-    });
-  } catch (e) {
-    throw new Error(e.name + ': ' + e.message);
-  }
+    values: [app.id, app.bungieApiKey, app.dimApiKey, app.origin],
+  });
 }

api/db/index.test.ts

@@ -10,6 +10,11 @@ beforeEach(async () => {
   )`);
 });
 
+interface TransactionTestRow {
+  id: number;
+  test: string;
+}
+
 afterAll(async () => {
   try {
     await pool.query(`DROP TABLE transaction_test`);
@@ -28,10 +33,10 @@ describe('transaction', () => {
       });
       fail('should have thrown an error');
     } catch (e) {
-      expect(e.message).toBe('oops');
+      expect((e as Error).message).toBe('oops');
     }
 
-    const result = await pool.query('select * from transaction_test');
+    const result = await pool.query<TransactionTestRow>('select * from transaction_test');
     expect(result.rows.length).toBe(1);
     expect(result.rows[0].id).toBe(1);
   });
@@ -41,7 +46,7 @@ describe('transaction', () => {
       await client.query("insert into transaction_test (id, test) values (3, 'testing commits')");
     });
 
-    const result = await pool.query('select * from transaction_test');
+    const result = await pool.query<TransactionTestRow>('select * from transaction_test');
     expect(result.rows.length).toBe(1);
     expect(result.rows[0].test).toBe('testing commits');
   });
@@ -61,7 +66,9 @@ describe('readTransaction', () => {
 
         // Now request that info from our original client.
         // should be read-committed, so we shouldn't see that update
-        const result = await client.query('select * from transaction_test where id = 1');
+        const result = await client.query<TransactionTestRow>(
+          'select * from transaction_test where id = 1',
+        );
         expect(result.rows[0].test).toBe('testing');
 
         // Commit the update
@@ -74,12 +81,16 @@ describe('readTransaction', () => {
       }
 
       // once that other transaction commits, we'll see its update
-      const result = await client.query('select * from transaction_test where id = 1');
+      const result = await client.query<TransactionTestRow>(
+        'select * from transaction_test where id = 1',
+      );
       expect(result.rows[0].test).toBe('updated');
     });
 
     // outside, we should still see the transactional update
-    const result = await pool.query('select * from transaction_test where id = 1');
+    const result = await pool.query<TransactionTestRow>(
+      'select * from transaction_test where id = 1',
+    );
     expect(result.rows[0].test).toBe('updated');
   });
 });

api/db/index.ts

@@ -21,7 +21,7 @@ pool.on('acquire', () => {
 });
 pool.on('error', (e: Error) => {
   metrics.increment('db.pool.error.count');
-  metrics.increment('db.pool.error.' + e.name + '.count');
+  metrics.increment(`db.pool.error.${  e.name  }.count`);
 });
 pool.on('remove', () => {
   metrics.increment('db.pool.remove.count');
@@ -66,10 +66,10 @@ export async function readTransaction<T>(fn: (client: ClientBase) => Promise<T>)
   const client = await pool.connect();
   try {
     // We used to wrap multiple reads in a transaction but I'm not sure it matters all that much.
-    //await client.query('BEGIN');
+    // await client.query('BEGIN');
     return await fn(client);
   } finally {
-    //await client.query('ROLLBACK');
+    // await client.query('ROLLBACK');
     client.release();
   }
 }