-
Notifications
You must be signed in to change notification settings - Fork 2
Rewrite type declarations #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,131 +1,158 @@ | ||||||
| type ToBuffer = (buffer: Buffer, value: unknown, index: number) => Buffer; | ||||||
| type FromBuffer = (buffer: Buffer, index: number, returnLength: boolean) => unknown; | ||||||
|
|
||||||
| declare module "@athombv/data-types" { | ||||||
| interface DataTypeInterface { | ||||||
| declare module "@athombv/data-types" { | ||||||
| class DataType<Value> { | ||||||
| id: number; | ||||||
| shortName: string; | ||||||
| length: number; | ||||||
| toBuffer: ToBuffer; | ||||||
| fromBuffer: FromBuffer; | ||||||
| args: unknown[]; | ||||||
| defaultValue: unknown; | ||||||
|
|
||||||
| isAnalog: () => boolean; | ||||||
| inspect: () => string; | ||||||
| } | ||||||
|
|
||||||
| interface DataTypeConstructor { | ||||||
| new ( | ||||||
| id: number, | ||||||
| shortName: string, | ||||||
| length: number, | ||||||
| toBuf: ToBuffer, | ||||||
| fromBuf: FromBuffer, | ||||||
| ...args: unknown[] | ||||||
| ): DataTypeInterface; | ||||||
| } | ||||||
|
|
||||||
| interface DataTypeFunctionConstructor { | ||||||
| (...args: unknown[]): DataTypeConstructor; | ||||||
| toBuffer: (buffer: Buffer, value: Value, index?: number) => number; | ||||||
| fromBuffer: (buffer: Buffer, index?: number) => Value; | ||||||
| args: Array<unknown>; | ||||||
| defaultValue: Value; | ||||||
|
|
||||||
| get isAnalog(): boolean; | ||||||
| inspect(): string; | ||||||
| } | ||||||
|
|
||||||
| type DataTypeItem = | ||||||
| | DataTypeConstructor | ||||||
| | DataTypeFunctionConstructor | ||||||
| | { [name: string]: DataTypeItem }; // This OR is needed for Structs with a second level of DataTypes | ||||||
|
|
||||||
| type GenericMap<T> = { | ||||||
| [K in keyof T]: DataTypeItem; | ||||||
|
|
||||||
| const DataTypes: { | ||||||
| noData: DataType<null>, | ||||||
|
|
||||||
| data8 : DataType<number>, | ||||||
| data16: DataType<number>, | ||||||
| data24: DataType<number>, | ||||||
| data32: DataType<number>, | ||||||
| data40: DataType<number>, | ||||||
| data48: DataType<number>, | ||||||
| data56: DataType<number>, | ||||||
|
Comment on lines
+18
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
|
||||||
| bool: DataType<boolean>, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| map8 : <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map16: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map24: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map32: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map40: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map48: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map56: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| map64: <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
|
Comment on lines
+28
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR types map8 as (flags: Array) - a single array argument. But node-zigbee calls it with spread args: |
||||||
|
|
||||||
| uint8 : DataType<number>, | ||||||
| uint16: DataType<number>, | ||||||
| uint24: DataType<number>, | ||||||
| uint32: DataType<number>, | ||||||
| uint40: DataType<number>, | ||||||
| uint48: DataType<number>, | ||||||
| // uint56: DataType<number>, | ||||||
| // uint64: DataType<number>, | ||||||
|
|
||||||
| int8 : DataType<number>, | ||||||
| int16: DataType<number>, | ||||||
| int24: DataType<number>, | ||||||
| int32: DataType<number>, | ||||||
| int40: DataType<number>, | ||||||
| int48: DataType<number>, | ||||||
| // int56: DataType<number>, | ||||||
| // int64: DataType<number>, | ||||||
|
|
||||||
| enum8 : <Flags extends string>(flags: Record<Flags, number>) => DataType<Flags>, | ||||||
| enum16: <Flags extends string>(flags: Record<Flags, number>) => DataType<Flags>, | ||||||
| enum32: <Flags extends string>(flags: Record<Flags, number>) => DataType<Flags>, | ||||||
|
Comment on lines
+55
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as the map types - the runtime uses (...arg) rest params, and some consumers spread additional args. In node-zigbee/zstack this causes TS2556: A spread argument must either have a tuple type or be passed to a rest parameter and TS2554: Expected 1 arguments, but got N errors. |
||||||
|
|
||||||
| // semi: DataType<number>, | ||||||
| single: DataType<number>, | ||||||
| double: DataType<number>, | ||||||
|
|
||||||
| octstr: DataType<string>, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. octstr typed as |
||||||
| string: DataType<string>, | ||||||
| // octstr16: DataType<string>, | ||||||
| // string16: DataType<string>, | ||||||
|
|
||||||
| // array | ||||||
| // struct | ||||||
| // set | ||||||
| // bag | ||||||
|
|
||||||
| // ToD | ||||||
| // date | ||||||
| // UTC | ||||||
|
|
||||||
| // clusterId | ||||||
| // attribId | ||||||
|
|
||||||
| // bacOID | ||||||
| EUI48 : DataType<string>, | ||||||
| EUI64 : DataType<string>, | ||||||
| key128: DataType<string>, | ||||||
|
|
||||||
| //* Internal Types *// | ||||||
| map4 : <Flags extends string | null>(flags: Array<Flags>) => DataType<BitMap<Flags>>, | ||||||
| uint4: DataType<number>, | ||||||
| enum4: <Flags extends string>(flags: Record<Flags, number>) => DataType<Flags>, | ||||||
|
|
||||||
| buffer : DataType<Buffer>, | ||||||
| buffer8 : DataType<Buffer>, | ||||||
| buffer16: DataType<Buffer>, | ||||||
|
|
||||||
| Array0: <Type>(type: Type) => DataType<Array<Type>>, | ||||||
| Array8: <Type>(type: Type) => DataType<Array<Type>>, | ||||||
|
Comment on lines
+94
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you call
|
||||||
| FixedString: (length: number) => DataType<string>, | ||||||
| }; | ||||||
|
|
||||||
| interface StructTypeInterface<T> { | ||||||
| toJSON: () => T; | ||||||
| toBuffer: (buffer?: Buffer, index?: number) => Buffer; | ||||||
|
|
||||||
| class BitMap<Flags extends string | null> { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| _buffer: Buffer; | ||||||
| _fields: Array<Flags>; | ||||||
| setBit(index: number, value: boolean): void; | ||||||
| getBit(index: number): boolean; | ||||||
| clearBit(index: number): void; | ||||||
| setBits(bits: number | Array<Flags>): void; | ||||||
| getBits(): Array<Flags>; | ||||||
| get length(): number; | ||||||
| static fromBuffer<Flags extends string | null>(buffer: Buffer, index: number, length: number, flags: Array<Flags>): BitMap<Flags>; | ||||||
| static toBuffer<Flags extends string | null>(buffer: Buffer, index: number, length: number, flags: Array<Flags>, value: number): number; | ||||||
| static toBuffer<Flags extends string | null>(buffer: Buffer, index: number, length: number, flags: Array<Flags> | undefined, value: BitMap<Flags>): number; | ||||||
| toArray(): Array<Flags>; | ||||||
| toBuffer(buffer: Buffer, index: number): Buffer; | ||||||
| copy(): BitMap<Flags>; | ||||||
| toJSON(): object; | ||||||
| inspect(): string; | ||||||
| } | ||||||
|
|
||||||
| export interface StructInstance<StructType> { | ||||||
| fromBuffer: (buffer: Buffer) => StructType & StructTypeInterface<StructType>; | ||||||
| toBuffer: (buffer: Buffer, object: StructType, index?: number) => Buffer; | ||||||
| fields: GenericMap<StructType>; | ||||||
| name: string; | ||||||
| length: number; | ||||||
| fromJSON: (object: StructType) => StructType & StructTypeInterface<StructType>; | ||||||
| fromArgs: (...args: unknown[]) => StructType & StructTypeInterface<StructType>; | ||||||
|
|
||||||
| interface StaticStruct<Defs extends Record<string, DataType<any>>> { | ||||||
| get fields(): Defs; | ||||||
| get name(): string; | ||||||
| get length(): number; | ||||||
| fromJSON(props: any): StructInstance<Defs>; | ||||||
| fromArgs(...args: Array<unknown>): StructInstance<Defs>; | ||||||
| fromBuffer(buffer: Buffer, index?: number, returnLength?: false): StructInstance<Defs>; | ||||||
| fromBuffer(buffer: Buffer, index?: number, returnLength?: true): { | ||||||
| result: StructInstance<Defs>, | ||||||
| length: number, | ||||||
| } | ||||||
| toBuffer(buffer: Buffer, value: StructInstance<Defs>, index?: number): number; | ||||||
| } | ||||||
| export const DataTypes: { | ||||||
| noData: DataTypeConstructor; | ||||||
| data8: DataTypeConstructor; | ||||||
| data16: DataTypeConstructor; | ||||||
| data24: DataTypeConstructor; | ||||||
| data32: DataTypeConstructor; | ||||||
| data40: DataTypeConstructor; | ||||||
| data48: DataTypeConstructor; | ||||||
| data56: DataTypeConstructor; | ||||||
| data64: DataTypeConstructor; | ||||||
| bool: DataTypeConstructor; | ||||||
| map8: DataTypeFunctionConstructor; | ||||||
| map16: DataTypeFunctionConstructor; | ||||||
| map24: DataTypeFunctionConstructor; | ||||||
| map32: DataTypeFunctionConstructor; | ||||||
| map40: DataTypeFunctionConstructor; | ||||||
| map48: DataTypeFunctionConstructor; | ||||||
| map56: DataTypeFunctionConstructor; | ||||||
| map64: DataTypeFunctionConstructor; | ||||||
| uint8: DataTypeConstructor; | ||||||
| uint16: DataTypeConstructor; | ||||||
| uint24: DataTypeConstructor; | ||||||
| uint32: DataTypeConstructor; | ||||||
| uint40: DataTypeConstructor; | ||||||
| uint48: DataTypeConstructor; | ||||||
| int8: DataTypeConstructor; | ||||||
| int16: DataTypeConstructor; | ||||||
| int24: DataTypeConstructor; | ||||||
| int32: DataTypeConstructor; | ||||||
| int40: DataTypeConstructor; | ||||||
| int48: DataTypeConstructor; | ||||||
| enum8: DataTypeFunctionConstructor; | ||||||
| enum16: DataTypeFunctionConstructor; | ||||||
| enum32: DataTypeFunctionConstructor; | ||||||
| single: DataTypeConstructor; | ||||||
| double: DataTypeConstructor; | ||||||
| octstr: DataTypeConstructor; | ||||||
| string: DataTypeConstructor; | ||||||
| EUI48: DataTypeConstructor; | ||||||
| EUI64: DataTypeConstructor; | ||||||
| key128: DataTypeConstructor; | ||||||
| uint4: DataTypeConstructor; | ||||||
| enum4: DataTypeFunctionConstructor; | ||||||
| map4: DataTypeFunctionConstructor; | ||||||
| buffer: DataTypeConstructor; | ||||||
| buffer8: DataTypeConstructor; | ||||||
| buffer16: DataTypeConstructor; | ||||||
| Array0: DataTypeFunctionConstructor; | ||||||
| Array8: DataTypeFunctionConstructor; | ||||||
| FixedString: DataTypeFunctionConstructor; | ||||||
| }; | ||||||
| export const DataType: DataTypeInterface; | ||||||
| export function Struct<StructType>( | ||||||
| name: string, | ||||||
| objectDefinition: GenericMap<StructType> | ||||||
| ): StructInstance<StructType>; | ||||||
|
|
||||||
| type StructInstance<Defs extends Record<string, import('@athombv/data-types').DataType<any>>> = StructProperties<Defs> & { | ||||||
| toJSON: () => StructProperties<Defs>; | ||||||
| toBuffer: (buffer: Buffer, index?: number) => Buffer; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Should be:
|
||||||
| } | ||||||
|
|
||||||
| function Struct<Defs extends Record<string, DataType<any>>> (name: string, defs: Defs, opts?: {encodeMissingFieldsBehavior?: 'default' | 'skip'}): StaticStruct<Defs>; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new generic parameter
This passes the output type as the generic, not the defs record. With this change, TS errors on every single Struct call (17+ errors across zigbee and zstack packages). |
||||||
| } | ||||||
|
|
||||||
| type StructProperties<Defs extends Record<string, import('@athombv/data-types').DataType<any>>> = { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a specific reason this is defined outside the module scope? |
||||||
| [Property in keyof Defs]: Defs[Property] extends import('@athombv/data-types').DataType<infer Type> ? Type : never | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| /* | ||||||
| How to use @athombv/data-types in TypeScript: | ||||||
|
|
||||||
| // Create a type that represents the Struct data | ||||||
| type ZdoEndDeviceAnnounceIndication = { | ||||||
| srcAddr: number; | ||||||
| IEEEAddr: string; | ||||||
| const ZdoEndDeviceAnnounceIndication = { | ||||||
| srcAddr: DataTypes.uint16, | ||||||
| IEEEAddr: DataTypes.EUI64, | ||||||
| }; | ||||||
|
|
||||||
| // Create a Struct instance with generic type ZdoEndDeviceAnnounceIndication | ||||||
| const ZdoEndDeviceAnnounceIndicationStruct = | ||||||
| Struct<ZdoEndDeviceAnnounceIndication>("ZdoEndDeviceAnnounceIndication", { | ||||||
| srcAddr: DataTypes.uint16, | ||||||
| IEEEAddr: DataTypes.EUI64, | ||||||
| }); | ||||||
| const ZdoEndDeviceAnnounceIndicationStruct = Struct("ZdoEndDeviceAnnounceIndication", ZdoEndDeviceAnnounceIndication); | ||||||
|
|
||||||
| // Create ZdoEndDeviceAnnounceIndication object | ||||||
| const ZdoEndDeviceAnnounceObject = ZdoEndDeviceAnnounceIndicationStruct.fromBuffer( | ||||||
|
|
@@ -135,9 +162,10 @@ const ZdoEndDeviceAnnounceObject = ZdoEndDeviceAnnounceIndicationStruct.fromBuff | |||||
| ZdoEndDeviceAnnounceObject.srcAddr.trim(); // This errors, srcAddr is not a string | ||||||
|
|
||||||
| // Create Buffer instance from ZdoEndDeviceAnnounceObject | ||||||
| const ZdoEndDeviceAnnounceBuffer = ZdoEndDeviceAnnounceIndicationStruct.toBuffer({ srcAddr: 1, IEEAddr: 'abc' }); // This errors due to typo in IEEEAddr name | ||||||
| const ZdoEndDeviceAnnounceBuffer = Buffer.alloc(8); | ||||||
| ZdoEndDeviceAnnounceIndicationStruct.toBuffer(ZdoEndDeviceAnnounceBuffer, { srcAddr: 1, IEEAddr: 'abc' }); // This errors due to typo in IEEEAddr name | ||||||
|
|
||||||
| Known limitations: | ||||||
| - Structs in Structs are considered a no-go by these definitions. | ||||||
| - DataTypes have no related JS type, so a few unknowns are used. | ||||||
| - Struct.fromArgs cannot be typed. | ||||||
| */ | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DataType constructor is not exposed in the type declarations, but consumers create custom DataType instances directly. For example in node-zigbee's zdoStructs.mts:
new DataType(NaN, 'ZdoComplexDescriptor', 0, toBufFn, fromBufFn)This currently errors with Expected 0 arguments, but got 5. The constructor needs to be declared:
constructor(id: number, shortName: string, length: number, toBuf: (...) => number, fromBuf: (...) => Value, ...args: unknown[]);