From 969837b908c6fe847c07e3732243d9ee867f560b Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 10 Nov 2022 16:40:15 +0100 Subject: [PATCH 1/2] Fix Record#get type checking 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 contructor 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 = // 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 nonExisitingKey = p.get('non-existing-key') ``` --- packages/core/src/record.ts | 14 +++---- packages/core/test/record.test.ts | 41 +++++++++++++++++++ packages/neo4j-driver-deno/lib/core/record.ts | 14 +++---- .../neo4j-driver/test/types/record.test.ts | 4 +- 4 files changed, 56 insertions(+), 17 deletions(-) diff --git a/packages/core/src/record.ts b/packages/core/src/record.ts index 21198436e..2358c6acd 100644 --- a/packages/core/src/record.ts +++ b/packages/core/src/record.ts @@ -187,24 +187,22 @@ class Record< return obj } - get(key: K): Entries[K] - get (key: keyof FieldLookup | number): any - + get (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 (key: K | number): any { + let index: number if (!(typeof key === 'number')) { + // @ts-expect-error 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 key are: [` + this.keys.toString() + '].' ) diff --git a/packages/core/test/record.test.ts b/packages/core/test/record.test.ts index 5f903ddc1..1e29315f5 100644 --- a/packages/core/test/record.test.ts +++ b/packages/core/test/record.test.ts @@ -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(['age', 'name'], [32, 'Dave']) + + // When & Then + expect(() => { + // @ts-expect-error + record.get('something') + }).toThrow( + newError( + "This record has no field with key 'something', available key 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 key 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 index dfdee2ee0..834e40801 100644 --- a/packages/neo4j-driver-deno/lib/core/record.ts +++ b/packages/neo4j-driver-deno/lib/core/record.ts @@ -187,24 +187,22 @@ class Record< return obj } - get(key: K): Entries[K] - get (key: keyof FieldLookup | number): any - + get (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 (key: K | number): any { + let index: number if (!(typeof key === 'number')) { + // @ts-expect-error 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 key are: [` + this.keys.toString() + '].' ) diff --git a/packages/neo4j-driver/test/types/record.test.ts b/packages/neo4j-driver/test/types/record.test.ts index 0c8ff1b48..7e4a9b2de 100644 --- 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') From a36b8b4c0f42cd93262bb7704255b6c9b1494c22 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Tue, 15 Nov 2022 11:15:35 +0100 Subject: [PATCH 2/2] Address comments in the PR --- packages/core/src/record.ts | 5 ++--- packages/core/test/record.test.ts | 6 +++--- packages/neo4j-driver-deno/lib/core/record.ts | 5 ++--- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/core/src/record.ts b/packages/core/src/record.ts index 2358c6acd..e10a7bcc3 100644 --- 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 = Dict + FieldLookup extends Dict = Dict > { keys: Key[] length: number @@ -198,11 +198,10 @@ class Record< get (key: K | number): any { let index: number if (!(typeof key === 'number')) { - // @ts-expect-error index = this._fieldLookup[key] if (index === undefined) { throw newError( - `This record has no field with key '${key.toString()}', 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 index 1e29315f5..47dde52e4 100644 --- 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]." ) ) }) @@ -211,7 +211,7 @@ describe('Record', () => { record.get('something') }).toThrow( newError( - "This record has no field with key 'something', available key are: [age,name]." + "This record has no field with key 'something', available keys are: [age,name]." ) ) @@ -229,7 +229,7 @@ describe('Record', () => { record.get('something') }).toThrow( newError( - "This record has no field with key 'something', available key are: [age,name]." + "This record has no field with key 'something', available keys are: [age,name]." ) ) diff --git a/packages/neo4j-driver-deno/lib/core/record.ts b/packages/neo4j-driver-deno/lib/core/record.ts index 834e40801..02a0a434e 100644 --- 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 = Dict + FieldLookup extends Dict = Dict > { keys: Key[] length: number @@ -198,11 +198,10 @@ class Record< get (key: K | number): any { let index: number if (!(typeof key === 'number')) { - // @ts-expect-error index = this._fieldLookup[key] if (index === undefined) { throw newError( - `This record has no field with key '${key.toString()}', available key are: [` + + `This record has no field with key '${key.toString()}', available keys are: [` + this.keys.toString() + '].' )