Add JsonSchema support to fastify plugin. by mitchknife · Pull Request #60 · FacilityApi/FacilityJavaScript

mitchknife · 2023-12-30T14:17:37Z

https://fastify.dev/docs/latest/Reference/Validation-and-Serialization/#serialization

I opted to only handle response schemas since (from my understanding) that is where most of the performance gain is at using fast-json-stringify. The request schemas are mostly for validation, which Facility can already take care of for us.

https://fastify.dev/docs/latest/Reference/Validation-and-Serialization/#serialization I opted to only handle response schemas since (from my understanding) that is where most of the performance gain is at using `fast-json-stringify`. The request schemas are mostly for validation, which Facility can already take care of for us.

ejball · 2023-12-30T16:09:21Z

src/Facility.CodeGen.JavaScript/JavaScriptGenerator.cs

+
+										if (response.BodyField is not null)
+										{
+											code.WriteLine($"{GetJsonSchemaType(response.BodyField.ServiceField.TypeName)},");


Suggested change

code.WriteLine($"{GetJsonSchemaType(response.BodyField.ServiceField.TypeName)},");

code.WriteLine($"{GetJsonSchemaType(service.GetFieldType(response.BodyField.ServiceField))},");

This would be a more typical implementation, which would allow GetJsonSchemaType to switch on fieldType.Kind instead of reverse-engineering the type name.

You beat me to it! I was going to ask if there was a better way, which I assumed there was. 👍

ejball · 2023-12-30T16:10:52Z

src/Facility.CodeGen.JavaScript/JavaScriptGenerator.cs

 			var capModuleName = CodeGenUtility.Capitalize(moduleName);
 			var camelCaseModuleName = CodeGenUtility.ToCamelCase(moduleName);
 			var pluginFileName = CodeGenUtility.Uncapitalize(moduleName) + "Plugin" + (TypeScript ? ".ts" : ".js");
+			var customTypes = new HashSet<string>(service.Dtos.Select(x => x.Name).Concat(service.Enums.Select(x => x.Name)));


I think when I add .NET 8 support, I'll drop .NET Standard 2.0 from the code gen libraries so they can use modern constructs like ToHashSet().

Sounds good!

instead of digging into the type name directly.

mitchknife · 2024-01-01T14:17:05Z

src/Facility.CodeGen.JavaScript/JavaScriptGenerator.cs

+				if (serviceType is null)
+					throw new ArgumentNullException(nameof(serviceType));


ServiceInfo.GetServiceType can return null, so I opted to handle that explicitly here instead of using ! at the call sites. @ejball What is the scenario where this will be null?

I think it would only be null if the type name was invalid, which shouldn't be possible if the service parsed successfully. Probably it should have thrown an exception on an invalid type and introduced a separate Try method for possibly returning null.

In any case, it doesn't seem correct to communicate that null is okay in the signature and then throw if it is passed.

Will change to use ! at the call sites then. Thanks!

fixed: 51f5c1f

ejball · 2024-01-01T18:31:23Z

src/Facility.CodeGen.JavaScript/JavaScriptGenerator.cs

+
+				return serviceType.Kind switch
+				{
+					ServiceTypeKind.String or ServiceTypeKind.Bytes or ServiceTypeKind.DateTime or ServiceTypeKind.ExternalEnum => $"{{ type: '{"string"}' }}",


Suggested change

ServiceTypeKind.String or ServiceTypeKind.Bytes or ServiceTypeKind.DateTime or ServiceTypeKind.ExternalEnum => $"{{ type: '{"string"}' }}",

ServiceTypeKind.String or ServiceTypeKind.Bytes or ServiceTypeKind.DateTime or ServiceTypeKind.ExternalEnum => "{ type: 'string' }",

So I did have this originally, but was getting a Probable JSON string detected (JSON002) linting error, which seemed strange. I'm not actually sure where it is coming from, I can't find anything online about it. I can disable it inline for this method, or maybe make it a suggestion instead of a an error?

Here's what I've found on that now: https://www.infoq.com/news/2022/11/visual-studio-tagged-strings/

Basically, I can add /*lang=json*/ to each line to let Roslyn check it for syntax errors. TIL.

I think the comments (in this particular case) hurt readability though, so I vote for adding dotnet_diagnostic.JSON002.severity = suggestion to .editorconfig.

fix: 580c68b

ejball · 2024-01-01T18:33:27Z

src/Facility.CodeGen.JavaScript/JavaScriptGenerator.cs

+					ServiceTypeKind.Error => $"{{ $ref: '{"_error"}' }}",
+					ServiceTypeKind.Dto => $"{{ $ref: '{serviceType.Dto!.Name}' }}",
+					ServiceTypeKind.Enum => $"{{ $ref: '{serviceType.Enum!.Name}' }}",
+					ServiceTypeKind.Result => $"{{ type: 'object', properties: {{ value: {GetJsonSchemaType(serviceType.ValueType!)}, error: {{ $ref: '_error' }} }} }}",


Suggested change

ServiceTypeKind.Result => $"{{ type: 'object', properties: {{ value: {GetJsonSchemaType(serviceType.ValueType!)}, error: {{ $ref: '_error' }} }} }}",

ServiceTypeKind.Result => $$"""{ type: 'object', properties: { value: {{GetJsonSchemaType(serviceType.ValueType!)}}, error: { $ref: '_error' } } }",

I can't tell if this is better or not...

You've got to double on braces somewhere I suppose. The current version is what I'm more familiar with as I don't see the """ syntax as much.

Instead we just set it to a suggestion in `.editorconfig`

mitchknife · 2024-01-02T22:52:07Z

@ejball 👍🏼 👎🏼 ?

ejball reviewed Dec 30, 2023

View reviewed changes

Switch on ServiceTypeKind

a00bbc0

instead of digging into the type name directly.

mitchknife commented Jan 1, 2024

View reviewed changes

ejball reviewed Jan 1, 2024

View reviewed changes

mitchknife added 2 commits January 1, 2024 14:20

Assume non-null value from GetFieldType.

51f5c1f

Don't use interpolated strings to prevent analysis warning.

580c68b

Instead we just set it to a suggestion in `.editorconfig`

ejball approved these changes Jan 2, 2024

View reviewed changes

mitchknife merged commit fff1953 into FacilityApi:master Jan 2, 2024

	code.WriteLine($"{GetJsonSchemaType(response.BodyField.ServiceField.TypeName)},");
	code.WriteLine($"{GetJsonSchemaType(service.GetFieldType(response.BodyField.ServiceField))},");

		if (serviceType is null)
		throw new ArgumentNullException(nameof(serviceType));

	ServiceTypeKind.String or ServiceTypeKind.Bytes or ServiceTypeKind.DateTime or ServiceTypeKind.ExternalEnum => $"{{ type: '{"string"}' }}",
	ServiceTypeKind.String or ServiceTypeKind.Bytes or ServiceTypeKind.DateTime or ServiceTypeKind.ExternalEnum => "{ type: 'string' }",

	ServiceTypeKind.Result => $"{{ type: 'object', properties: {{ value: {GetJsonSchemaType(serviceType.ValueType!)}, error: {{ $ref: '_error' }} }} }}",
	ServiceTypeKind.Result => $$"""{ type: 'object', properties: { value: {{GetJsonSchemaType(serviceType.ValueType!)}}, error: { $ref: '_error' } } }",

Conversation

mitchknife commented Dec 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchknife Jan 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ejball Jan 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchknife Jan 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchknife commented Jan 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mitchknife Jan 1, 2024 •

edited

Loading

ejball Jan 1, 2024 •

edited

Loading

mitchknife Jan 1, 2024 •

edited

Loading