Skip to content

Commit

Permalink
fix: Use jsonName even with snakeToCamel=false. (#653)
Browse files Browse the repository at this point in the history
Previously we only checked jsonName if snakeToCamel=json was enabled,
b/c protobuf _always_ set jsonName to the camelized key name OR the
jsonName attribute value.

Because of this, we couldn't do logic like "if jsonName is explicitly
set, use that, otherwise keep the field name as-is".

Now we guess about whether jsonName is explicitly set by doing our
own snake-field-name -> camel-json-name, and if our camel-json-name
matches jsonName, then we assume the user has _not_ set jsonName,
and we should keep using snake-field-name.

But if those two values, camel-json-name and jsonName, are different,
then the user has probably manually set jsonName in the proto file,
and we'll use that instead.

Fixes #635
  • Loading branch information
stephenh authored Aug 14, 2022
1 parent aaa6c86 commit 1144886
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 19 deletions.
33 changes: 17 additions & 16 deletions src/case.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
import { Options } from './options';

export function maybeSnakeToCamel(s: string, options: Pick<Options, 'snakeToCamel'>): string {
if (options.snakeToCamel.includes('keys') && s.includes('_')) {
const hasLowerCase = !!s.match(/[a-z]/);
return s
.split('_')
.map((word, i) => {
// If the word is already mixed case, leave the exist case as-is
word = hasLowerCase ? word : word.toLowerCase();
return i === 0 ? word : capitalize(word);
})
.join('');
/** Converts `key` to TS/JS camel-case idiom, unless overridden not to. */
export function maybeSnakeToCamel(key: string, options: Pick<Options, 'snakeToCamel'>): string {
if (options.snakeToCamel.includes('keys') && key.includes('_')) {
return snakeToCamel(key);
} else {
return s;
return key;
}
}

export function camelToSnake(s: string): string {
export function snakeToCamel(s: string): string {
const hasLowerCase = !!s.match(/[a-z]/);
return s
.replace(/[\w]([A-Z])/g, function (m) {
return m[0] + '_' + m[1];
.split('_')
.map((word, i) => {
// If the word is already mixed case, leave the existing case as-is
word = hasLowerCase ? word : word.toLowerCase();
return i === 0 ? word : capitalize(word);
})
.toUpperCase();
.join('');
}

export function camelToSnake(s: string): string {
return s.replace(/\w([A-Z])/g, (m) => m[0] + '_' + m[1]).toUpperCase();
}

export function capitalize(s: string): string {
Expand Down
13 changes: 10 additions & 3 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import ReadStream = NodeJS.ReadStream;
import { SourceDescription } from './sourceInfo';
import { Options, ServiceOption } from './options';
import { camelCase } from './case';
import { camelCase, snakeToCamel } from './case';

export function protoFilesToGenerate(request: CodeGeneratorRequest): FileDescriptorProto[] {
return request.protoFile.filter((f) => request.fileToGenerate.includes(f.name));
Expand Down Expand Up @@ -174,13 +174,20 @@ export class FormattedMethodDescriptor implements MethodDescriptorProto {
}
}

export function getFieldJsonName(field: FieldDescriptorProto, options: Options): string {
export function getFieldJsonName(
field: Pick<FieldDescriptorProto, 'name' | 'jsonName'>,
options: Pick<Options, 'snakeToCamel'>
): string {
// jsonName will be camelCased by the protocol compiler, plus can be overridden by the user,
// so just use that instead of our own maybeSnakeToCamel
if (options.snakeToCamel.includes('json')) {
return field.jsonName;
} else {
return field.name;
// The user wants to keep snake case in the JSON, but we still want to see if the jsonName
// attribute is set as an explicit override.
const probableJsonName = snakeToCamel(field.name);
const isJsonNameSet = probableJsonName !== field.jsonName;
return isJsonNameSet ? field.jsonName : field.name;
}
}

Expand Down
11 changes: 11 additions & 0 deletions tests/case-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { maybeSnakeToCamel } from '../src/case';
import { Options, optionsFromParameter } from '../src/options';
import { getFieldJsonName } from '../src/utils';

const keys = optionsFromParameter('snakeToCamel=keys');

Expand Down Expand Up @@ -45,4 +46,14 @@ describe('case', () => {
it('converts snake to camel with first underscore and camelize other', () => {
expect(maybeSnakeToCamel('_uuid_foo', { snakeToCamel: ['keys'] })).toEqual('UuidFoo');
});

describe('getFieldJsonName', () => {
it('keeps snake case when jsonName is probably not set', () => {
expect(getFieldJsonName({ name: 'foo_bar', jsonName: 'fooBar' }, { snakeToCamel: [] })).toBe('foo_bar');
});

it('uses jsonName when it is set', () => {
expect(getFieldJsonName({ name: 'foo_bar', jsonName: 'foo' }, { snakeToCamel: [] })).toBe('foo');
});
});
});

0 comments on commit 1144886

Please sign in to comment.