diff --git a/.prettierrc b/.prettierrc index 963354f23..73e070e80 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,3 +1,4 @@ { - "printWidth": 120 -} + "printWidth": 120, + "trailingComma": "all" +} \ No newline at end of file diff --git a/integration/simple-long/simple-test.ts b/integration/simple-long/simple-test.ts index 5842c90af..453821f3c 100644 --- a/integration/simple-long/simple-test.ts +++ b/integration/simple-long/simple-test.ts @@ -1,3 +1,4 @@ +import * as Long from "long"; import { SimpleWithMap } from "./simple"; describe("simple", () => { @@ -6,7 +7,7 @@ describe("simple", () => { expect(s1).toMatchInlineSnapshot(` { "intLookup": {}, - "longLookup": {}, + "longLookup": Map {}, "nameLookup": {}, } `); @@ -22,7 +23,7 @@ describe("simple", () => { "1": 2, "2": 1, }, - "longLookup": {}, + "longLookup": Map {}, "nameLookup": {}, } `); @@ -31,21 +32,31 @@ describe("simple", () => { it("can fromPartial maps", () => { const s1 = SimpleWithMap.fromPartial({ intLookup: { 1: 2, 2: 1 }, - longLookup: { "1": 2, "2": 1 }, + longLookup: new Map(), }); + s1.longLookup.set(Long.fromInt(1), Long.fromInt(2)); + s1.longLookup.set(Long.fromInt(2), Long.fromInt(1)); expect(s1).toMatchInlineSnapshot(` { "intLookup": { "1": 2, "2": 1, }, - "longLookup": { - "1": Long { + "longLookup": Map { + Long { + "high": 0, + "low": 1, + "unsigned": false, + } => Long { "high": 0, "low": 2, "unsigned": false, }, - "2": Long { + Long { + "high": 0, + "low": 2, + "unsigned": false, + } => Long { "high": 0, "low": 1, "unsigned": false, @@ -59,8 +70,10 @@ describe("simple", () => { it("can toJSON/fromJSON maps", () => { const s1 = SimpleWithMap.fromPartial({ intLookup: { 1: 2, 2: 1 }, - longLookup: { "1": 2, "2": 1 }, + longLookup: new Map(), }); + s1.longLookup.set(Long.fromInt(1), Long.fromInt(2)); + s1.longLookup.set(Long.fromInt(2), Long.fromInt(1)); const json = SimpleWithMap.toJSON(s1); expect(json).toMatchInlineSnapshot(` @@ -83,13 +96,21 @@ describe("simple", () => { "1": 2, "2": 1, }, - "longLookup": { - "1": Long { + "longLookup": Map { + Long { + "high": 0, + "low": 1, + "unsigned": false, + } => Long { "high": 0, "low": 2, "unsigned": false, }, - "2": Long { + Long { + "high": 0, + "low": 2, + "unsigned": false, + } => Long { "high": 0, "low": 1, "unsigned": false, diff --git a/integration/simple-long/simple.bin b/integration/simple-long/simple.bin index 28e779a04..88f2622ab 100644 Binary files a/integration/simple-long/simple.bin and b/integration/simple-long/simple.bin differ diff --git a/integration/simple-long/simple.proto b/integration/simple-long/simple.proto index 9c812fc70..e7684065b 100644 --- a/integration/simple-long/simple.proto +++ b/integration/simple-long/simple.proto @@ -14,8 +14,7 @@ message SimpleWithWrappers { message SimpleWithMap { map nameLookup = 2; map intLookup = 3; - // Ideally we'd test map but we present maps as JS objects and `Long` cannot be used as a keys. - map longLookup = 4; + map longLookup = 4; } message Numbers { diff --git a/integration/simple-long/simple.ts b/integration/simple-long/simple.ts index e48f71c54..e58ccda17 100644 --- a/integration/simple-long/simple.ts +++ b/integration/simple-long/simple.ts @@ -17,8 +17,7 @@ export interface SimpleWithWrappers { export interface SimpleWithMap { nameLookup: { [key: string]: string }; intLookup: { [key: number]: number }; - /** Ideally we'd test map but we present maps as JS objects and `Long` cannot be used as a keys. */ - longLookup: { [key: string]: Long }; + longLookup: Map; } export interface SimpleWithMap_NameLookupEntry { @@ -32,7 +31,7 @@ export interface SimpleWithMap_IntLookupEntry { } export interface SimpleWithMap_LongLookupEntry { - key: string; + key: Long; value: Long; } @@ -190,7 +189,7 @@ export const SimpleWithWrappers = { }; function createBaseSimpleWithMap(): SimpleWithMap { - return { nameLookup: {}, intLookup: {}, longLookup: {} }; + return { nameLookup: {}, intLookup: {}, longLookup: new Map() }; } export const SimpleWithMap = { @@ -201,7 +200,7 @@ export const SimpleWithMap = { Object.entries(message.intLookup).forEach(([key, value]) => { SimpleWithMap_IntLookupEntry.encode({ key: key as any, value }, writer.uint32(26).fork()).ldelim(); }); - Object.entries(message.longLookup).forEach(([key, value]) => { + (message.longLookup).forEach((value, key) => { SimpleWithMap_LongLookupEntry.encode({ key: key as any, value }, writer.uint32(34).fork()).ldelim(); }); return writer; @@ -241,7 +240,7 @@ export const SimpleWithMap = { const entry4 = SimpleWithMap_LongLookupEntry.decode(reader, reader.uint32()); if (entry4.value !== undefined) { - message.longLookup[entry4.key] = entry4.value; + message.longLookup.set(entry4.key, entry4.value); } continue; } @@ -268,11 +267,11 @@ export const SimpleWithMap = { }, {}) : {}, longLookup: isObject(object.longLookup) - ? Object.entries(object.longLookup).reduce<{ [key: string]: Long }>((acc, [key, value]) => { - acc[key] = Long.fromValue(value as Long | string); + ? Object.entries(object.longLookup).reduce>((acc, [key, value]) => { + acc.set(Long.fromValue(key), Long.fromValue(value as Long | string)); return acc; - }, {}) - : {}, + }, new Map()) + : new Map(), }; }, @@ -296,14 +295,11 @@ export const SimpleWithMap = { }); } } - if (message.longLookup) { - const entries = Object.entries(message.longLookup); - if (entries.length > 0) { - obj.longLookup = {}; - entries.forEach(([k, v]) => { - obj.longLookup[k] = v.toString(); - }); - } + if (message.longLookup?.size) { + obj.longLookup = {}; + message.longLookup.forEach((v, k) => { + obj.longLookup[longToNumber(k)] = v.toString(); + }); } return obj; }, @@ -331,15 +327,15 @@ export const SimpleWithMap = { }, {}, ); - message.longLookup = Object.entries(object.longLookup ?? {}).reduce<{ [key: string]: Long }>( - (acc, [key, value]) => { + message.longLookup = (() => { + const m = new Map(); + (object.longLookup as Map ?? new Map()).forEach((value, key) => { if (value !== undefined) { - acc[key] = Long.fromValue(value); + m.set(key, Long.fromValue(value)); } - return acc; - }, - {}, - ); + }); + return m; + })(); return message; }, }; @@ -489,13 +485,13 @@ export const SimpleWithMap_IntLookupEntry = { }; function createBaseSimpleWithMap_LongLookupEntry(): SimpleWithMap_LongLookupEntry { - return { key: "", value: Long.ZERO }; + return { key: Long.ZERO, value: Long.ZERO }; } export const SimpleWithMap_LongLookupEntry = { encode(message: SimpleWithMap_LongLookupEntry, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { - if (message.key !== "") { - writer.uint32(10).string(message.key); + if (!message.key.isZero()) { + writer.uint32(8).int64(message.key); } if (!message.value.isZero()) { writer.uint32(16).int64(message.value); @@ -511,11 +507,11 @@ export const SimpleWithMap_LongLookupEntry = { const tag = reader.uint32(); switch (tag >>> 3) { case 1: - if (tag !== 10) { + if (tag !== 8) { break; } - message.key = reader.string(); + message.key = reader.int64() as Long; continue; case 2: if (tag !== 16) { @@ -535,15 +531,15 @@ export const SimpleWithMap_LongLookupEntry = { fromJSON(object: any): SimpleWithMap_LongLookupEntry { return { - key: isSet(object.key) ? String(object.key) : "", + key: isSet(object.key) ? Long.fromValue(object.key) : Long.ZERO, value: isSet(object.value) ? Long.fromValue(object.value) : Long.ZERO, }; }, toJSON(message: SimpleWithMap_LongLookupEntry): unknown { const obj: any = {}; - if (message.key !== "") { - obj.key = message.key; + if (!message.key.isZero()) { + obj.key = (message.key || Long.ZERO).toString(); } if (!message.value.isZero()) { obj.value = (message.value || Long.ZERO).toString(); @@ -558,7 +554,7 @@ export const SimpleWithMap_LongLookupEntry = { object: I, ): SimpleWithMap_LongLookupEntry { const message = createBaseSimpleWithMap_LongLookupEntry(); - message.key = object.key ?? ""; + message.key = (object.key !== undefined && object.key !== null) ? Long.fromValue(object.key) : Long.ZERO; message.value = (object.value !== undefined && object.value !== null) ? Long.fromValue(object.value) : Long.ZERO; return message; }, @@ -837,6 +833,25 @@ export const Numbers = { }, }; +declare const self: any | undefined; +declare const window: any | undefined; +declare const global: any | undefined; +const tsProtoGlobalThis: any = (() => { + if (typeof globalThis !== "undefined") { + return globalThis; + } + if (typeof self !== "undefined") { + return self; + } + if (typeof window !== "undefined") { + return window; + } + if (typeof global !== "undefined") { + return global; + } + throw "Unable to locate global object"; +})(); + type Builtin = Date | Function | Uint8Array | string | number | boolean | undefined; export type DeepPartial = T extends Builtin ? T @@ -849,6 +864,13 @@ type KeysOfUnion = T extends T ? keyof T : never; export type Exact = P extends Builtin ? P : P & { [K in keyof P]: Exact } & { [K in Exclude>]: never }; +function longToNumber(long: Long): number { + if (long.gt(Number.MAX_SAFE_INTEGER)) { + throw new tsProtoGlobalThis.Error("Value is larger than Number.MAX_SAFE_INTEGER"); + } + return long.toNumber(); +} + if (_m0.util.Long !== Long) { _m0.util.Long = Long as any; _m0.configure(); diff --git a/package.json b/package.json index 07ef71902..9ab93bceb 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "mongodb": "^5.7.0", "nice-grpc": "^2.1.4", "object-hash": "^3.0.0", - "prettier": "^3.0.0", + "prettier": "^2.8.8", "protobufjs-cli": "^1.1.1", "reflect-metadata": "^0.1.13", "rxjs": "^7.8.1", diff --git a/src/enums.ts b/src/enums.ts index f77a8cc75..6c727eae4 100644 --- a/src/enums.ts +++ b/src/enums.ts @@ -41,8 +41,8 @@ export function generateEnum( if (options.unrecognizedEnum) chunks.push(code` ${UNRECOGNIZED_ENUM_NAME} ${delimiter} ${ - options.stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString() - },`); + options.stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString() + },`); if (options.enumsAsLiterals) { chunks.push(code`} as const`); diff --git a/src/generate-services.ts b/src/generate-services.ts index e741e511f..3d3402e28 100644 --- a/src/generate-services.ts +++ b/src/generate-services.ts @@ -309,8 +309,8 @@ function generateCachingRpcMethod( const responses = requests.map(async request => { const data = ${inputType}.encode(request).finish() const response = await this.rpc.request(ctx, "${maybePrefixPackage(fileDesc, serviceDesc.name)}", "${ - methodDesc.name - }", data); + methodDesc.name + }", data); return ${outputType}.decode(${Reader}.create(response)); }); return Promise.all(responses); diff --git a/src/main.ts b/src/main.ts index 769354ee2..9f143fe53 100644 --- a/src/main.ts +++ b/src/main.ts @@ -42,6 +42,7 @@ import { packedType, toReaderCall, toTypeName, + shouldGenerateJSMapType, valueTypeName, } from "./types"; import SourceInfo, { Fields } from "./sourceInfo"; @@ -960,7 +961,7 @@ function generateBaseInstanceFactory( const val = isWithinOneOf(field) ? "undefined" : isMapType(ctx, messageDesc, field) - ? ctx.options.useMapType + ? shouldGenerateJSMapType(ctx, messageDesc, field) ? "new Map()" : "{}" : isRepeated(field) @@ -1098,17 +1099,22 @@ function generateDecode(ctx: Context, fullName: string, messageDesc: DescriptorP if (isRepeated(field)) { const maybeNonNullAssertion = ctx.options.useOptionals === "all" ? "!" : ""; - if (isMapType(ctx, messageDesc, field)) { + const mapType = detectMapType(ctx, messageDesc, field); + if (mapType) { // We need a unique const within the `cast` statement const varName = `entry${field.number}`; + const generateMapType = shouldGenerateJSMapType(ctx, messageDesc, field); - const valueSetterSnippet = ctx.options.useMapType - ? `message.${fieldName}${maybeNonNullAssertion}.set(${varName}.key, ${varName}.value)` - : `message.${fieldName}${maybeNonNullAssertion}[${varName}.key] = ${varName}.value`; + let valueSetterSnippet: string; + if (generateMapType) { + valueSetterSnippet = `message.${fieldName}${maybeNonNullAssertion}.set(${varName}.key, ${varName}.value)`; + } else { + valueSetterSnippet = `message.${fieldName}${maybeNonNullAssertion}[${varName}.key] = ${varName}.value`; + } const initializerSnippet = initializerNecessary ? ` if (message.${fieldName} === undefined) { - message.${fieldName} = ${ctx.options.useMapType ? "new Map()" : "{}"}; + message.${fieldName} = ${generateMapType ? "new Map()" : "{}"}; }` : ""; chunks.push(code` @@ -1314,9 +1320,10 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP } ` : writeSnippet(`{ ${maybeTypeField} key: key as any, value }`); - const optionalAlternative = isOptional ? (ctx.options.useMapType ? " || new Map()" : " || {}") : ""; + const useMapType = shouldGenerateJSMapType(ctx, messageDesc, field); + const optionalAlternative = isOptional ? (useMapType ? " || new Map()" : " || {}") : ""; - if (ctx.options.useMapType) { + if (useMapType) { chunks.push(code` (message.${fieldName}${optionalAlternative}).forEach((value, key) => { ${entryWriteSnippet} @@ -1833,7 +1840,7 @@ function generateFromJson(ctx: Context, fullName: string, fullTypeName: string, const fieldType = toTypeName(ctx, messageDesc, field); const i = convertFromObjectKey(ctx, messageDesc, field, "key"); - if (ctx.options.useMapType) { + if (shouldGenerateJSMapType(ctx, messageDesc, field)) { const fallback = noDefaultValue ? "undefined" : "new Map()"; chunks.push(code` @@ -2023,7 +2030,7 @@ function generateToJson( // Maps might need their values transformed, i.e. bytes --> base64 const i = convertToObjectKey(ctx, messageDesc, field, "k"); - if (ctx.options.useMapType) { + if (shouldGenerateJSMapType(ctx, messageDesc, field)) { chunks.push(code` if (message.${fieldName}?.size) { ${jsonProperty} = {}; @@ -2189,7 +2196,7 @@ function generateFromPartial(ctx: Context, fullName: string, messageDesc: Descri ? `(object.${fieldName} === undefined || object.${fieldName} === null) ? undefined : ` : ""; - if (ctx.options.useMapType) { + if (shouldGenerateJSMapType(ctx, messageDesc, field)) { chunks.push(code` message.${fieldName} = ${noValueSnippet} (() => { const m = new Map(); @@ -2261,7 +2268,7 @@ function convertFromObjectKey( const { keyType, keyField } = detectMapType(ctx, messageDesc, field)!; if (keyType.toCodeString([]) === "string") { return code`${variableName}`; - } else if (isLong(keyField) && ctx.options.useMapType) { + } else if (isLong(keyField) && shouldGenerateJSMapType(ctx, messageDesc, field)) { if (ctx.options.forceLong === LongOption.LONG) { return code`${capitalize(keyType.toCodeString([]))}.fromValue(${variableName})`; } else if (ctx.options.forceLong === LongOption.BIGINT) { @@ -2285,7 +2292,7 @@ function convertToObjectKey( const { keyType, keyField } = detectMapType(ctx, messageDesc, field)!; if (keyType.toCodeString([]) === "string") { return code`${variableName}`; - } else if (isLong(keyField) && ctx.options.useMapType) { + } else if (isLong(keyField) && shouldGenerateJSMapType(ctx, messageDesc, field)) { if (ctx.options.forceLong === LongOption.LONG) { return code`${ctx.utils.longToNumber}(${variableName})`; } else if (ctx.options.forceLong === LongOption.BIGINT) { diff --git a/src/types.ts b/src/types.ts index 26c98a9cc..d3722e75e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -620,7 +620,7 @@ export function toTypeName( const mapType = messageDesc ? detectMapType(ctx, messageDesc, field) : false; if (mapType) { const { keyType, valueType } = mapType; - if (ctx.options.useMapType) { + if (shouldGenerateJSMapType(ctx, messageDesc!, field)) { return finalize(code`Map<${keyType}, ${valueType}>`, ensureOptional); } return finalize(code`{ [key: ${keyType} ]: ${valueType} }`, ensureOptional); @@ -660,6 +660,29 @@ export function toTypeName( ); } +/** + * For a protobuf map field, if the generated code should use the javascript Map type. + * + * If the type of a protobuf map key corresponds to the Long type, we always use the Map type. This avoids generating + * invalid code such as below (using Long as key of a javascript object): + * + * export interface Foo { + * bar: { [key: Long]: Long } + * } + * + * See https://github.com/stephenh/ts-proto/issues/708 for more details. + */ +export function shouldGenerateJSMapType(ctx: Context, message: DescriptorProto, field: FieldDescriptorProto): boolean { + if (ctx.options.useMapType) { + return true; + } + const mapType = detectMapType(ctx, message, field); + if (!mapType) { + return false; + } + return isLong(mapType.keyField) && ctx.options.forceLong === LongOption.LONG; +} + export function detectMapType( ctx: Context, messageDesc: DescriptorProto, diff --git a/yarn.lock b/yarn.lock index 298adf0de..c25b14787 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6622,12 +6622,12 @@ __metadata: languageName: node linkType: hard -"prettier@npm:^3.0.0": - version: 3.0.0 - resolution: "prettier@npm:3.0.0" +"prettier@npm:^2.8.8": + version: 2.8.8 + resolution: "prettier@npm:2.8.8" bin: - prettier: bin/prettier.cjs - checksum: 6a832876a1552dc58330d2467874e5a0b46b9ccbfc5d3531eb69d15684743e7f83dc9fbd202db6270446deba9c82b79d24383d09924c462b457136a759425e33 + prettier: bin-prettier.js + checksum: b49e409431bf129dd89238d64299ba80717b57ff5a6d1c1a8b1a28b590d998a34e083fa13573bc732bb8d2305becb4c9a4407f8486c81fa7d55100eb08263cf8 languageName: node linkType: hard @@ -7854,7 +7854,7 @@ __metadata: mongodb: ^5.7.0 nice-grpc: ^2.1.4 object-hash: ^3.0.0 - prettier: ^3.0.0 + prettier: ^2.8.8 protobufjs: ^7.2.4 protobufjs-cli: ^1.1.1 reflect-metadata: ^0.1.13