feat(execute): Remove the need to call withArgs after the execute fun…

…ction BREAKING CHANGE: The withArgs function has been removed. Previously it has been required so that type safety around the query arguments can be ensured. Now args must be passed in as a second parameter of the execute function. This change also effects how the query function is structured. Now, the “args” are passed into the second argument of the query function. Here is an example of how it now works: const query = queryExecutor.createQuery<string, string>( async ({ tables }, arg) => { tables.tableOne() return arg } ) await queryExecutor.execute(query, ‘hello!’)
sevenwestmedia-labs · Jan 24, 2019 · 9d26436 · 9d26436
1 parent 9f475f3
commit 9d26436
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 68 deletions.
diff --git a/README.md b/README.md
@@ -63,7 +63,7 @@ interface QueryResult {
 const exampleQuery = queryExecutor.createQuery(async function exampleQuery<
     QueryArgs,
     QueryResult
->({ args, tables, tableNames, query }) {
+>({ tables, tableNames, query }, args) {
     // You can access the query arguments through `args`
     const { someArg } = args
 
@@ -79,7 +79,7 @@ const exampleQuery = queryExecutor.createQuery(async function exampleQuery<
 })
 
 // Then execute the query
-const queryResult = await queryExecutor.execute(exampleQuery).withArgs({})
+const queryResult = await queryExecutor.execute(exampleQuery, { someArg: 'pass the args as the second parameter' })
 ```
 
 ### Wrapping database queries

diff --git a/src/__snapshots__/type-safety.test.ts.snap b/src/__snapshots__/type-safety.test.ts.snap
@@ -1,10 +1,18 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`Typescript: Typescript expected failures 1`] = `
-"src/type-safety-fixtures/create-query-helper.ts(17,20): error TS2339: Property 'wrongTable' does not exist on type 'TableNames<\\"testTable\\">'.
+"src/type-safety-fixtures/args-respected.ts(37,27): error TS7006: Parameter '_' implicitly has an 'any' type.
+src/type-safety-fixtures/args-respected.ts(45,41): error TS2345: Argument of type '1' is not assignable to parameter of type 'object'.
+src/type-safety-fixtures/args-respected.ts(46,41): error TS2345: Argument of type '1' is not assignable to parameter of type 'string'.
+src/type-safety-fixtures/args-respected.ts(47,41): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ id: number; }'.
+  Property 'id' is missing in type '{}' but required in type '{ id: number; }'.
+src/type-safety-fixtures/args-respected.ts(48,41): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ id: number; }'.
+  Property 'id' is missing in type '{}' but required in type '{ id: number; }'.
+src/type-safety-fixtures/args-respected.ts(50,11): error TS2554: Expected 2 arguments, but got 1.
+src/type-safety-fixtures/create-query-helper.ts(17,20): error TS2339: Property 'wrongTable' does not exist on type 'TableNames<\\"testTable\\">'.
 src/type-safety-fixtures/create-query-helper.ts(18,16): error TS2339: Property 'wrongTable' does not exist on type 'Tables<\\"testTable\\">'.
 src/type-safety-fixtures/create-query-helper.ts(20,21): error TS2339: Property 'foo' does not exist on type '{ testArg: string; }'.
-src/type-safety-fixtures/query-arguments.ts(22,46): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ testArg: string; }'.
+src/type-safety-fixtures/query-arguments.ts(22,37): error TS2345: Argument of type '{}' is not assignable to parameter of type '{ testArg: string; }'.
   Property 'testArg' is missing in type '{}' but required in type '{ testArg: string; }'.
 "
 `;
diff --git a/src/index.test.ts b/src/index.test.ts
@@ -12,7 +12,7 @@ it('can initialise read query executor', async () => {
         async ({ tableNames }) => tableNames.tableOne
     )
 
-    const tableName = await queryExecutor.execute(tableNameQuery).withArgs({})
+    const tableName = await queryExecutor.execute(tableNameQuery, {})
 
     expect(tableName).toBe('table-one')
 })
@@ -32,7 +32,7 @@ it('can wrap queryBuilder queries', async () => {
         tables.tableOne()
     )
 
-    await queryExecutor.execute(tableNameQuery).withArgs({})
+    await queryExecutor.execute(tableNameQuery, {})
 
     expect(executedQuery).toEqual('select * from `table-one`')
 })
@@ -52,7 +52,7 @@ it('can wrap raw queries', async () => {
         query(db => db.raw('select 1'))
     )
 
-    await queryExecutor.execute(testQuery).withArgs({})
+    await queryExecutor.execute(testQuery, {})
 
     expect(executedQuery).toEqual('select 1')
 })
@@ -75,7 +75,7 @@ it('combined wrap API can wrap builder queries', async () => {
         tables.tableOne()
     )
 
-    await queryExecutor.execute(testQuery).withArgs({})
+    await queryExecutor.execute(testQuery, {})
 
     expect(executedQuery).toEqual('select * from `table-one`')
 })
@@ -98,7 +98,7 @@ it('combined wrap API can wrap raw queries', async () => {
         query(db => db.raw('select 1'))
     )
 
-    await queryExecutor.execute(testQuery).withArgs({})
+    await queryExecutor.execute(testQuery, {})
 
     expect(executedQuery).toEqual('select 1')
 })
diff --git a/src/index.ts b/src/index.ts
@@ -21,12 +21,9 @@ export type TableNames<TTableNames extends string> = {
 }
 
 type QueryOptions<
-    QueryArguments,
     TTableNames extends string,
     Services extends {}
 > = Services & {
-    args: QueryArguments
-
     /**
      * Gives raw access to knex, while still allowing the query to be
      * tracked/benchmarked
@@ -38,18 +35,18 @@ type QueryOptions<
     tableNames: TableNames<TTableNames>
     queryExecutor: QueryExecutor<TTableNames, Services>
 }
+
 export type Query<
     QueryArguments,
     QueryResult,
     TTableNames extends string,
     Services extends object
 > = (
-    options: QueryOptions<QueryArguments, TTableNames, Services>
+    options: QueryOptions<TTableNames, Services>,
+    args: QueryArguments
 ) => PromiseLike<QueryResult>
 
-export interface ExecuteResult<Args, Result> {
-    withArgs: (args: Args) => Promise<Result>
-}
+export type ExecuteResult<Result> = PromiseLike<Result>
 
 export interface QueryWrapperFn {
     (builder: Knex.QueryBuilder): Knex.QueryBuilder
@@ -64,3 +61,11 @@ export type QueryWrapper =
               builder: Knex.QueryBuilder
           ) => Knex.QueryBuilder
       }
+
+export type GetQueryArgs<T> = T extends Query<infer P, any, any, any>
+    ? P
+    : never
+
+export type GetQueryResult<T> = T extends Query<any, infer P, any, any>
+    ? P
+    : never
diff --git a/src/mock-query-executor.test.ts b/src/mock-query-executor.test.ts
@@ -16,7 +16,7 @@ it('can mock query', async () => {
     })
 
     // Execute the query as normal
-    const result = await queryExecutor.execute(exampleQuery).withArgs({})
+    const result = await queryExecutor.execute(exampleQuery, {})
 
     expect(result).toEqual([1])
 })
@@ -38,12 +38,10 @@ it('can match specific query args', async () => {
     })
 
     // Execute the query as normal
-    const result = await queryExecutor
-        .execute(exampleQuery)
-        .withArgs({ param: 'first' })
-    const result2 = await queryExecutor
-        .execute(exampleQuery)
-        .withArgs({ param: 'other' })
+    const result = await queryExecutor.execute(exampleQuery, { param: 'first' })
+    const result2 = await queryExecutor.execute(exampleQuery, {
+        param: 'other'
+    })
 
     expect(result).toEqual(1)
     expect(result2).toEqual(2)

diff --git a/src/mock-query-executor.ts b/src/mock-query-executor.ts
@@ -1,4 +1,4 @@
-import { ExecuteResult, Query, TableNames } from '.'
+import { ExecuteResult, GetQueryArgs, GetQueryResult, Query } from '.'
 import { ReadQueryExecutor } from './read-query-executor'
 import { UnitOfWorkQueryExecutor } from './unit-of-work-query-executor'
 export const NoMatch = Symbol('no match')
@@ -57,30 +57,26 @@ export class MockQueryExecutor extends ReadQueryExecutor<any, any> {
         return mocker
     }
 
-    execute<Args, Result>(
-        query: Query<Args, Result, any, any>
-    ): ExecuteResult<Args, Result> {
-        return {
-            withArgs: (args: Args) => {
-                for (const mock of this.mocks) {
-                    if (mock.query === query) {
-                        const matcherResult = mock.matcher(args)
-                        if (matcherResult !== NoMatch) {
-                            return new Promise(resolve => {
-                                // Using setTimeout so this is not synchronous
-                                setTimeout(() => {
-                                    resolve(matcherResult)
-                                }, 0)
-                            })
-                        }
-                    }
+    execute<Q extends Query<any, any, any, any>>(
+        query: Q,
+        args: GetQueryArgs<Q>
+    ): ExecuteResult<GetQueryResult<Q>> {
+        for (const mock of this.mocks) {
+            if (mock.query === query) {
+                const matcherResult = mock.matcher(args)
+                if (matcherResult !== NoMatch) {
+                    return new Promise(resolve => {
+                        // Using setTimeout so this is not synchronous
+                        setTimeout(() => {
+                            resolve(matcherResult)
+                        }, 0)
+                    })
                 }
-
-                throw new Error(
-                    `No matcher for query ${query.name || 'unnamed function'}`
-                )
             }
         }
+        throw new Error(
+            `No matcher for query ${query.name || 'unnamed function'}`
+        )
     }
 
     /**

diff --git a/src/query-executor.ts b/src/query-executor.ts
@@ -1,5 +1,13 @@
 import Knex from 'knex'
-import { ExecuteResult, Query, QueryWrapper, TableNames, Tables } from '.'
+import {
+    ExecuteResult,
+    GetQueryArgs,
+    GetQueryResult,
+    TableNames,
+    Tables,
+    Query,
+    QueryWrapper
+} from '.'
 
 export class QueryExecutor<
     TTableNames extends string,
@@ -27,28 +35,29 @@ export class QueryExecutor<
     }
 
     /** Helper to create type safe queries */
-    createQuery<QueryArguments, QueryResult>(
+    createQuery<QueryArguments = object, QueryResult = any>(
         query: Query<QueryArguments, QueryResult, TTableNames, Services>
-    ): Query<QueryArguments, QueryResult, TTableNames, Services> {
+    ) {
         return query
     }
 
-    execute<Args, Result>(
-        query: Query<Args, Result, TTableNames, Services>
-    ): ExecuteResult<Args, Result> {
-        return {
-            withArgs: async args =>
-                query({
-                    query: getQuery => {
-                        return performWrap(getQuery(this.knex), this.wrapQuery)
-                    },
-                    queryExecutor: this,
-                    tables: this.tables,
-                    args,
-                    tableNames: this.tableNames,
-                    ...this.services
-                })
-        }
+    execute<Q extends Query<any, any, TTableNames, Services>>(
+        query: Q,
+        args: GetQueryArgs<Q>
+    ): ExecuteResult<GetQueryResult<Q>> {
+        return query(
+            {
+                query: getQuery => {
+                    return performWrap(getQuery(this.knex), this.wrapQuery)
+                },
+                queryExecutor: this,
+                tables: this.tables,
+                args,
+                tableNames: this.tableNames,
+                ...this.services
+            },
+            args
+        )
     }
 }
 

diff --git a/src/type-safety-fixtures/args-respected.ts b/src/type-safety-fixtures/args-respected.ts
@@ -0,0 +1,55 @@
+import { ReadQueryExecutor } from '../'
+import { createMockedKnex } from '../test-helpers/knex'
+
+const testTables = {
+    tableOne: 'table-one'
+}
+
+export async function dontComplainAboutUnused() {
+    let executedQuery: any
+
+    const knex = createMockedKnex(query => query.response([]))
+    const queryExecutor = new ReadQueryExecutor(knex, {}, testTables, {
+        queryBuilderWrapper: query => {
+            executedQuery = query.toString()
+            return query
+        }
+    })
+
+    const query2 = async (_: any, args: string) => {
+        return args
+    }
+
+    const query1 = queryExecutor.createQuery(async ({ tables }, args) => {
+        ;(() => args)()
+        tables.tableOne()
+        return {}
+    })
+
+    const query3 = queryExecutor.createQuery<{ id: number }, {}>(
+        async ({ tables }, args) => {
+            ;(() => args)()
+            tables.tableOne()
+            return {}
+        }
+    )
+
+    const query4 = async (_, args: { id: number }) => {
+        return {}
+    }
+
+    const query5 = async () => {
+        return {}
+    }
+
+    await queryExecutor.execute(query1, 1)
+    await queryExecutor.execute(query2, 1)
+    await queryExecutor.execute(query3, {})
+    await queryExecutor.execute(query4, {})
+    await queryExecutor.execute(query5, {})
+    await queryExecutor.execute(query5)
+
+    await queryExecutor.execute(async () => 'x', {})
+
+    return executedQuery
+}
diff --git a/src/type-safety-fixtures/create-query-helper.ts b/src/type-safety-fixtures/create-query-helper.ts
@@ -13,7 +13,7 @@ const queryExecutor = new ReadQueryExecutor(
 
 const exampleQuery = queryExecutor.createQuery<{ testArg: string }, string>(
     // tslint:disable-next-line:no-shadowed-variable
-    async function exampleQuery({ args, tableNames, tables }) {
+    async function exampleQuery({ tableNames, tables }, args) {
         tableNames.wrongTable
         tables.wrongTable()
 

diff --git a/src/type-safety-fixtures/query-arguments.ts b/src/type-safety-fixtures/query-arguments.ts
@@ -14,9 +14,9 @@ const exampleQuery: Query<
     string,
     keyof typeof tableNames,
     {}
-> = async function exampleQuery({ args }) {
+> = async function exampleQuery(_, args) {
     return args.testArg
 }
 
 // Should fail to compile with missing testArg
-queryExecutor.execute(exampleQuery).withArgs({})
+queryExecutor.execute(exampleQuery, {})
diff --git a/tslint.json b/tslint.json
@@ -9,6 +9,7 @@
         "object-literal-sort-keys": false,
         "member-access": false,
         "interface-name": false,
-        "no-implicit-dependencies": false
+        "no-implicit-dependencies": false,
+        "ordered-imports": false
     }
 }