Skip to content

Commit 1fe1471

Browse files
authored
Fix Record#get type checking (#1015)
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.
1 parent 85c0939 commit 1fe1471

File tree

4 files changed

+57
-20
lines changed

4 files changed

+57
-20
lines changed

packages/core/src/record.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ function generateFieldLookup<
6868
class Record<
6969
Entries extends Dict = Dict,
7070
Key extends keyof Entries = keyof Entries,
71-
FieldLookup extends Dict<string, number> = Dict<string, number>
71+
FieldLookup extends Dict<keyof Entries, number> = Dict<keyof Entries, number>
7272
> {
7373
keys: Key[]
7474
length: number
@@ -187,24 +187,21 @@ class Record<
187187
return obj
188188
}
189189

190-
get<K extends Key>(key: K): Entries[K]
191-
get (key: keyof FieldLookup | number): any
192-
190+
get <K extends keyof Entries = keyof Entries> (key: K): Entries[K]
191+
get (n: number): any
193192
/**
194193
* Get a value from this record, either by index or by field key.
195194
*
196195
* @param {string|Number} key Field key, or the index of the field.
197196
* @returns {*}
198197
*/
199-
get (key: string | number): any {
200-
let index
198+
get <K extends keyof Entries = keyof Entries> (key: K | number): any {
199+
let index: number
201200
if (!(typeof key === 'number')) {
202201
index = this._fieldLookup[key]
203202
if (index === undefined) {
204203
throw newError(
205-
"This record has no field with key '" +
206-
key +
207-
"', available key are: [" +
204+
`This record has no field with key '${key.toString()}', available keys are: [` +
208205
this.keys.toString() +
209206
'].'
210207
)

packages/core/test/record.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('Record', () => {
6565
record.get('age')
6666
}).toThrow(
6767
newError(
68-
"This record has no field with key 'age', available key are: [name]."
68+
"This record has no field with key 'age', available keys are: [name]."
6969
)
7070
)
7171
})
@@ -195,4 +195,45 @@ describe('Record', () => {
195195
// Then
196196
expect(values).toEqual(['Bob', 45])
197197
})
198+
199+
it('should be able call .get() and use the field types', () => {
200+
// Given
201+
interface Person {
202+
age: number
203+
name: string
204+
}
205+
206+
const record = new Record<Person>(['age', 'name'], [32, 'Dave'])
207+
208+
// When & Then
209+
expect(() => {
210+
// @ts-expect-error
211+
record.get('something')
212+
}).toThrow(
213+
newError(
214+
"This record has no field with key 'something', available keys are: [age,name]."
215+
)
216+
)
217+
218+
expect(record.get('age')).toBe(32)
219+
expect(record.get('name')).toBe('Dave')
220+
})
221+
222+
it('should be able call .get() with plain string', () => {
223+
// Given
224+
225+
const record: Record = new Record(['age', 'name'], [32, 'Dave'])
226+
227+
// When & Then
228+
expect(() => {
229+
record.get('something')
230+
}).toThrow(
231+
newError(
232+
"This record has no field with key 'something', available keys are: [age,name]."
233+
)
234+
)
235+
236+
expect(record.get('age')).toBe(32)
237+
expect(record.get('name')).toBe('Dave')
238+
})
198239
})

packages/neo4j-driver-deno/lib/core/record.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ function generateFieldLookup<
6868
class Record<
6969
Entries extends Dict = Dict,
7070
Key extends keyof Entries = keyof Entries,
71-
FieldLookup extends Dict<string, number> = Dict<string, number>
71+
FieldLookup extends Dict<keyof Entries, number> = Dict<keyof Entries, number>
7272
> {
7373
keys: Key[]
7474
length: number
@@ -187,24 +187,21 @@ class Record<
187187
return obj
188188
}
189189

190-
get<K extends Key>(key: K): Entries[K]
191-
get (key: keyof FieldLookup | number): any
192-
190+
get <K extends keyof Entries = keyof Entries> (key: K): Entries[K]
191+
get (n: number): any
193192
/**
194193
* Get a value from this record, either by index or by field key.
195194
*
196195
* @param {string|Number} key Field key, or the index of the field.
197196
* @returns {*}
198197
*/
199-
get (key: string | number): any {
200-
let index
198+
get <K extends keyof Entries = keyof Entries> (key: K | number): any {
199+
let index: number
201200
if (!(typeof key === 'number')) {
202201
index = this._fieldLookup[key]
203202
if (index === undefined) {
204203
throw newError(
205-
"This record has no field with key '" +
206-
key +
207-
"', available key are: [" +
204+
`This record has no field with key '${key.toString()}', available keys are: [` +
208205
this.keys.toString() +
209206
'].'
210207
)

packages/neo4j-driver/test/types/record.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,11 @@ const record2Get2: string[] = record2.get('age')
7979

8080
const record3Get1: string = record3.get('name')
8181
const record3Get2: number = record3.get('age')
82+
const record3Get3: any = record3.get(0)
83+
const record3Get4: any = record3.get(1)
8284

8385
const record2Get3: string = record2.get('firstName')
8486
const record2Get4: number = record2.get(1)
8587

8688
// @ts-expect-error
87-
const record2Get5: any = record2.get('does-not-exist')
89+
const record3Get5: any = record3.get('does-not-exist')

0 commit comments

Comments
 (0)