Skip to content
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

PropertyNameGenerator: add passing and failing tests demonstrating issue with CodeGeneration #4711

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnnyreilly
Copy link
Contributor

@johnnyreilly johnnyreilly commented Jan 15, 2024

As mentioned here: #1874 (comment)

I've identified an issue with the TypeScript code generation of clients when using the PropertyNameGenerator property to control naming.

Essentially, when using TypeScriptTypeStyle.Class this is respected. When using TypeScriptTypeStyle.Interface it is ignored. This is illustrated by the enclosing tests, one of which fails; demonstrating the issue.

I'd love to help get this fixed and I figured this was a first step!

I've also added a C# test as well as I thought it'd be useful for coverage.

With the following CustomPropertyNameGenerator we can demostrate the issue.

        class CustomPropertyNameGenerator : NJsonSchema.CodeGeneration.IPropertyNameGenerator {
            public virtual string Generate(JsonSchemaProperty property) => $"XX_{property.Name}";
        }

If this is working as expected, all property names should be prefixed with XX_:

export interface IPerson {
    XX_FirstName: string;
    XX_LastName?: string | undefined;
    XX_Birthday?: Date | undefined;
    XX_Sex?: Sex | undefined;
    XX_Address?: Address | undefined;
}

But if using TypeScriptTypeStyle.Interface it is ignored and instead this is generated:

export interface Person {
    FirstName: string;
    LastName?: string | undefined;
    Birthday?: Date | undefined;
    Sex?: Sex | undefined;
    Address?: Address | undefined;
}

Ideally the CustomPropertyNameGenerator should always be in play.


Having had a dig through the code I wonder if the issue lies in NJsonSchema somewhere:

https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptTypeResolver.cs
https://github.com/RicoSuter/NJsonSchema/blob/3585d60e949e43284601e0bea16c33de4c6c21f5/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptGeneratorSettings.cs#L36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant