fix: simplify handling useJsonWireFormat=true and fix onlyTypes=true (#…

…583)

* Review: Moved logic for required and implied options into options.ts

* Make onlyTypes=true behave as documented.

* Rephrased.

* Added tests.

* Formatting.

README.markdown

@@ -396,7 +396,7 @@ Generated code will be placed in the Gradle build directory.
 
 - With `--ts_proto_opt=onlyTypes=true`, only types will be emitted, and imports for `long` and `protobufjs/minimal` will be excluded.
 
-  Note: _This is a combination_ of `outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false`
+  This is the same as setting `outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false`
 
 - With `--ts_proto_opt=usePrototypeForDefaults=true`, the generated code will wrap new objects with `Object.create`.
 
@@ -411,12 +411,6 @@ Generated code will be placed in the Gradle build directory.
   Requires `onlyTypes=true`. Implies `useDate=string` and `stringEnums=true`. This option is to generate types that can be directly used with marshalling/unmarshalling Protobuf messages serialized as JSON.  
   You may also want to set `useOptionals=all`, as gRPC gateways are not required to send default value for scalar values.
 
-### Only Types
-
-If you're looking for `ts-proto` to generate only types for your Protobuf types then passing all three of `outputEncodeMethods`, `outputJsonMethods`, and `outputClientImpl` as `false` is probably what you want, i.e.:
-
-`--ts_proto_opt=onlyTypes=true`.
-
 ### NestJS Support
 
 We have a great way of working together with [nestjs](https://docs.nestjs.com/microservices/grpc). `ts-proto` generates `interfaces` and `decorators` for you controller, client. For more information see the [nestjs readme](NESTJS.markdown).

src/enums.ts

@@ -27,20 +27,19 @@ export function generateEnum(
   }
 
   const delimiter = options.enumsAsLiterals ? ':' : '=';
-  const stringEnums = options.stringEnums || (options.onlyTypes && options.useJsonWireFormat);
 
   enumDesc.value.forEach((valueDesc, index) => {
     const info = sourceInfo.lookup(Fields.enum.value, index);
     maybeAddComment(info, chunks, valueDesc.options?.deprecated, `${valueDesc.name} - `);
     chunks.push(
-      code`${valueDesc.name} ${delimiter} ${stringEnums ? `"${valueDesc.name}"` : valueDesc.number.toString()},`
+      code`${valueDesc.name} ${delimiter} ${options.stringEnums ? `"${valueDesc.name}"` : valueDesc.number.toString()},`
     );
   });
 
   if (options.unrecognizedEnum)
     chunks.push(code`
       ${UNRECOGNIZED_ENUM_NAME} ${delimiter} ${
-      stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString()
+      options.stringEnums ? `"${UNRECOGNIZED_ENUM_NAME}"` : UNRECOGNIZED_ENUM_VALUE.toString()
     },`);
 
   if (options.enumsAsLiterals) {
@@ -51,15 +50,15 @@ export function generateEnum(
     chunks.push(code`}`);
   }
 
-  if (options.outputJsonMethods || (stringEnums && options.outputEncodeMethods)) {
+  if (options.outputJsonMethods || (options.stringEnums && options.outputEncodeMethods)) {
     chunks.push(code`\n`);
     chunks.push(generateEnumFromJson(ctx, fullName, enumDesc));
   }
   if (options.outputJsonMethods) {
     chunks.push(code`\n`);
     chunks.push(generateEnumToJson(ctx, fullName, enumDesc));
   }
-  if (stringEnums && options.outputEncodeMethods) {
+  if (options.stringEnums && options.outputEncodeMethods) {
     chunks.push(code`\n`);
     chunks.push(generateEnumToNumber(ctx, fullName, enumDesc));
   }

src/options.ts

@@ -58,7 +58,6 @@ export type Options = {
   unrecognizedEnum: boolean;
   exportCommonSymbols: boolean;
   outputSchema: boolean;
-  // An alias of !output
   onlyTypes: boolean;
   emitImportedFiles: boolean;
   useExactTypes: boolean;
@@ -124,8 +123,18 @@ export function optionsFromParameter(parameter: string | undefined): Options {
     }
     Object.assign(options, parsed);
   }
-  // We should promote onlyTypes to its own documented flag, but just an alias for now
-  if (!options.outputJsonMethods && !options.outputEncodeMethods && !options.outputClientImpl && !options.nestJs) {
+  // onlyTypes=true implies outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false,nestJs=false
+  if (options.onlyTypes) {
+    options.outputJsonMethods = false;
+    options.outputEncodeMethods = false;
+    options.outputClientImpl = false;
+    options.nestJs = false;
+  } else if (
+    !options.outputJsonMethods &&
+    !options.outputEncodeMethods &&
+    !options.outputClientImpl &&
+    !options.nestJs
+  ) {
     options.onlyTypes = true;
   }
 
@@ -164,6 +173,17 @@ export function optionsFromParameter(parameter: string | undefined): Options {
     options.snakeToCamel = [options.snakeToCamel];
   }
 
+  if (options.useJsonWireFormat) {
+    if (!options.onlyTypes) {
+      // useJsonWireFormat requires onlyTypes=true
+      options.useJsonWireFormat = false;
+    } else {
+      // useJsonWireFormat implies stringEnums=true and useDate=string
+      options.stringEnums = true;
+      options.useDate = DateOption.STRING;
+    }
+  }
+
   return options;
 }
 

src/types.ts

@@ -462,7 +462,6 @@ export function isEmptyType(typeName: string): boolean {
 }
 
 export function valueTypeName(ctx: Context, typeName: string): Code | undefined {
-  const useJsonWireFormat = ctx.options.onlyTypes && ctx.options.useJsonWireFormat;
   switch (typeName) {
     case '.google.protobuf.StringValue':
       return code`string`;
@@ -478,19 +477,23 @@ export function valueTypeName(ctx: Context, typeName: string): Code | undefined
     case '.google.protobuf.BoolValue':
       return code`boolean`;
     case '.google.protobuf.BytesValue':
-      return ctx.options.env === EnvOption.NODE ? code`Buffer` : useJsonWireFormat ? code`string` : code`Uint8Array`;
+      return ctx.options.env === EnvOption.NODE
+        ? code`Buffer`
+        : ctx.options.useJsonWireFormat
+        ? code`string`
+        : code`Uint8Array`;
     case '.google.protobuf.ListValue':
       return code`Array<any>`;
     case '.google.protobuf.Value':
       return code`any`;
     case '.google.protobuf.Struct':
       return code`{[key: string]: any}`;
     case '.google.protobuf.FieldMask':
-      return useJsonWireFormat ? code`string` : code`string[]`;
+      return ctx.options.useJsonWireFormat ? code`string` : code`string[]`;
     case '.google.protobuf.Duration':
-      return useJsonWireFormat ? code`string` : undefined;
+      return ctx.options.useJsonWireFormat ? code`string` : undefined;
     case '.google.protobuf.Timestamp':
-      return useJsonWireFormat ? code`string` : undefined;
+      return ctx.options.useJsonWireFormat ? code`string` : undefined;
     default:
       return undefined;
   }

tests/options-test.ts

@@ -1,4 +1,4 @@
-import { optionsFromParameter, ServiceOption } from '../src/options';
+import { DateOption, optionsFromParameter, ServiceOption } from '../src/options';
 
 describe('options', () => {
   it('can set outputJsonMethods with nestJs=true', () => {
@@ -99,4 +99,38 @@ describe('options', () => {
     const options = optionsFromParameter('foo=one,foo=two');
     expect(options).toMatchObject({ foo: ['one', 'two'] });
   });
+
+  it('´output*=false implies onlyTypes', () => {
+    const options = optionsFromParameter('outputJsonMethods=false,outputEncodeMethods=false,outputClientImpl=false');
+    expect(options).toMatchObject({
+      onlyTypes: true,
+    });
+  });
+
+  it('onlyTypes implies output*=false', () => {
+    const options = optionsFromParameter('onlyTypes=true');
+    expect(options).toMatchObject({
+      outputJsonMethods: false,
+      outputEncodeMethods: false,
+      outputClientImpl: false,
+      nestJs: false,
+    });
+  });
+
+  it('useJsonWireFormat requires onlyTypes', () => {
+    const options = optionsFromParameter('useJsonWireFormat=true');
+    expect(options).toMatchObject({
+      useJsonWireFormat: false,
+    });
+  });
+
+  it('useJsonWireFormat implies useDate=string and stringEnums=true', () => {
+    const options = optionsFromParameter('useJsonWireFormat=true,onlyTypes=true');
+    expect(options).toMatchObject({
+      useJsonWireFormat: true,
+      onlyTypes: true,
+      stringEnums: true,
+      useDate: DateOption.STRING,
+    });
+  });
 });