Fix Record#get type checking (#1015)

bigmontz · web-flow · commit 1fe14718c3e2 · 2022-11-15T12:18:17.000+01:00
This method was not correctly type checking the key argument. Keys are not in the Entries where being accepted without trigger typescript errors. The error was being cause because the method was not relying in the keys set in the Entries, but the ones came from the constructor and from the field lookup. The keys came from constructor and field lookup are not meant to be used in the client code, since they are internal. This way, the error was solving by strict `get` method key type for consider only indexes (number) and Key originated from the Entries. Example: ```typescript interface Person { age: Integer name: string } const p: Record<Person> = // get record form somewhere const age: Integer = p.get('age') const name: string = p.get('name') // @ts-expected-error This error was not being point out before const nonExistingKey = p.get('non-existing-key') ``` ⚠️ This type definitions are not asserted in runtime. Thus mismatched type records coming from the database will not trigger type errors.
diff --git a/packages/core/src/record.ts b/packages/core/src/record.ts
@@ -68,7 +68,7 @@ function generateFieldLookup<
 class Record<
   Entries extends Dict = Dict,
   Key extends keyof Entries = keyof Entries,
-  FieldLookup extends Dict<string, number> = Dict<string, number>
+  FieldLookup extends Dict<keyof Entries, number> = Dict<keyof Entries, number>
 > {
   keys: Key[]
   length: number
@@ -187,24 +187,21 @@ class Record<
     return obj
   }
 
-  get<K extends Key>(key: K): Entries[K]
-  get (key: keyof FieldLookup | number): any
-
+  get <K extends keyof Entries = keyof Entries> (key: K): Entries[K]
+  get (n: number): any
   /**
    * Get a value from this record, either by index or by field key.
    *
    * @param {string|Number} key Field key, or the index of the field.
    * @returns {*}
    */
-  get (key: string | number): any {
-    let index
+  get <K extends keyof Entries = keyof Entries> (key: K | number): any {
+    let index: number
     if (!(typeof key === 'number')) {
       index = this._fieldLookup[key]
       if (index === undefined) {
         throw newError(
-          "This record has no field with key '" +
-            key +
-            "', available key are: [" +
+          `This record has no field with key '${key.toString()}', available keys are: [` +
             this.keys.toString() +
             '].'
         )
diff --git a/packages/core/test/record.test.ts b/packages/core/test/record.test.ts
@@ -65,7 +65,7 @@ describe('Record', () => {
       record.get('age')
     }).toThrow(
       newError(
-        "This record has no field with key 'age', available key are: [name]."
+        "This record has no field with key 'age', available keys are: [name]."
       )
     )
   })
@@ -195,4 +195,45 @@ describe('Record', () => {
     // Then
     expect(values).toEqual(['Bob', 45])
   })
+
+  it('should be able call .get() and use the field types', () => {
+    // Given
+    interface Person {
+      age: number
+      name: string
+    }
+
+    const record = new Record<Person>(['age', 'name'], [32, 'Dave'])
+
+    // When & Then
+    expect(() => {
+      // @ts-expect-error
+      record.get('something')
+    }).toThrow(
+      newError(
+        "This record has no field with key 'something', available keys are: [age,name]."
+      )
+    )
+
+    expect(record.get('age')).toBe(32)
+    expect(record.get('name')).toBe('Dave')
+  })
+
+  it('should be able call .get() with plain string', () => {
+    // Given
+
+    const record: Record = new Record(['age', 'name'], [32, 'Dave'])
+
+    // When & Then
+    expect(() => {
+      record.get('something')
+    }).toThrow(
+      newError(
+        "This record has no field with key 'something', available keys are: [age,name]."
+      )
+    )
+
+    expect(record.get('age')).toBe(32)
+    expect(record.get('name')).toBe('Dave')
+  })
 })
diff --git a/packages/neo4j-driver-deno/lib/core/record.ts b/packages/neo4j-driver-deno/lib/core/record.ts
@@ -68,7 +68,7 @@ function generateFieldLookup<
 class Record<
   Entries extends Dict = Dict,
   Key extends keyof Entries = keyof Entries,
-  FieldLookup extends Dict<string, number> = Dict<string, number>
+  FieldLookup extends Dict<keyof Entries, number> = Dict<keyof Entries, number>
 > {
   keys: Key[]
   length: number
@@ -187,24 +187,21 @@ class Record<
     return obj
   }
 
-  get<K extends Key>(key: K): Entries[K]
-  get (key: keyof FieldLookup | number): any
-
+  get <K extends keyof Entries = keyof Entries> (key: K): Entries[K]
+  get (n: number): any
   /**
    * Get a value from this record, either by index or by field key.
    *
    * @param {string|Number} key Field key, or the index of the field.
    * @returns {*}
    */
-  get (key: string | number): any {
-    let index
+  get <K extends keyof Entries = keyof Entries> (key: K | number): any {
+    let index: number
     if (!(typeof key === 'number')) {
       index = this._fieldLookup[key]
       if (index === undefined) {
         throw newError(
-          "This record has no field with key '" +
-            key +
-            "', available key are: [" +
+          `This record has no field with key '${key.toString()}', available keys are: [` +
             this.keys.toString() +
             '].'
         )
diff --git a/packages/neo4j-driver/test/types/record.test.ts b/packages/neo4j-driver/test/types/record.test.ts
@@ -79,9 +79,11 @@ const record2Get2: string[] = record2.get('age')
 
 const record3Get1: string = record3.get('name')
 const record3Get2: number = record3.get('age')
+const record3Get3: any = record3.get(0)
+const record3Get4: any = record3.get(1)
 
 const record2Get3: string = record2.get('firstName')
 const record2Get4: number = record2.get(1)
 
 // @ts-expect-error
-const record2Get5: any = record2.get('does-not-exist')
+const record3Get5: any = record3.get('does-not-exist')
