Skip to content

Commit

Permalink
fix: Unbreak a use case for #1110 / fix for #1121 (#1123)
Browse files Browse the repository at this point in the history
"Function" is builtin, but anything prefixed or suffixed for whatever
reason shouldn't be handled the same way. So check if the final result
is in builtin list, not the original unmodified name

I have concerns about maybeSnakeToCamel being called earlier now, but i
can't find any tests for options.snakeToCamel.includes("keys") so i'm
not sure if i broke anything

```
  /** 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 key;
    }
  }
```

But all tests pass
  • Loading branch information
halkeye authored Oct 15, 2024
1 parent b4e3012 commit 476e99b
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 6 deletions.
7 changes: 6 additions & 1 deletion integration/test-1110/test-1110-test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { UserRule } from "./test-1110";
import { UserRule, Nested_Function } from "./test-1110";

describe("test-1110", () => {
it("Able to create a partial user rule with reserved word messages", () => {
const simple: UserRule = UserRule.fromPartial({ UUID: "foo" });
expect(simple).toBeTruthy();
});

it("built in handling should only be done to top level", () => {
const nestedFunction: Nested_Function = Nested_Function.fromPartial({});
expect(nestedFunction).toBeTruthy();
});
});
7 changes: 7 additions & 0 deletions integration/test-1110/test-1110.proto
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,12 @@ message Function {
string name = 2;
}

message Nested {
message Function {
string namespace = 1;
string name = 2;
}
}

service APIService {
}
127 changes: 127 additions & 0 deletions integration/test-1110/test-1110.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions src/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function visit(
const protoFullName = protoPrefix + enumDesc.name;
// I.e. FooBar_ZazInner
const tsFullName = tsPrefix + maybeSnakeToCamel(enumDesc.name, options);
const tsFullNameWithAffixes = `${options.typePrefix}${tsFullName}${options.typeSuffix}`;
const tsFullNameWithAffixes = messageName(`${options.typePrefix}${tsFullName}${options.typeSuffix}`);
const nestedSourceInfo = sourceInfo.open(childEnumType, index);
enumFn(tsFullNameWithAffixes, enumDesc, nestedSourceInfo, protoFullName);
});
Expand All @@ -51,8 +51,8 @@ export function visit(
// I.e. Foo_Bar.Zaz_Inner
const protoFullName = protoPrefix + message.name;
// I.e. FooBar_ZazInner
const tsFullName = tsPrefix + maybeSnakeToCamel(messageName(message), options);
const tsFullNameWithAffixes = `${options.typePrefix}${tsFullName}${options.typeSuffix}`;
const tsFullName = tsPrefix + maybeSnakeToCamel(message.name, options);
const tsFullNameWithAffixes = messageName(`${options.typePrefix}${tsFullName}${options.typeSuffix}`);
const nestedSourceInfo = sourceInfo.open(childType, index);
messageFn(tsFullNameWithAffixes, message, nestedSourceInfo, protoFullName);
const delim = options.useSnakeTypeName ? "_" : "";
Expand All @@ -63,8 +63,7 @@ export function visit(
const builtInNames = ["Date", "Function"];

/** Potentially suffixes `Message` to names to avoid conflicts, i.e. with `Date`. */
function messageName(message: DescriptorProto): string {
const { name } = message;
function messageName(name: string): string {
return builtInNames.includes(name) ? `${name}Message` : name;
}

Expand Down

0 comments on commit 476e99b

Please sign in to comment.