[http-server-csharp] add arrayDeclarationContext#10327
[http-server-csharp] add arrayDeclarationContext#10327sophia-ramsey wants to merge 2 commits intomainfrom
Conversation
|
All changed packages have been documented.
|
|
You can try these changes here
|
commit: |
215dc6d to
91d3430
Compare
markcowl
left a comment
There was a problem hiding this comment.
Thanks for the contribution! A few comments. I think the context is necessary, but the dedicated emitted file is not useful and we should avoid emitting it if possible.
| `, | ||
| [ | ||
| [ | ||
| "Tags.cs", |
There was a problem hiding this comment.
Why are we generating a file for this? What do we expect the file to contain? Shouldn't we find a way to not emit the array type (while making sure we emit the item type)?
| "My.Custom.Ns", | ||
| ], | ||
| [ | ||
| ["Items.cs", ["// Generated by @typespec/http-server-csharp", "using System;"]], |
| "public string Name { get; set; }", | ||
| ], | ||
| ], | ||
| ["WidgetList.cs", ["// Generated by @typespec/http-server-csharp", "using System;"]], |
There was a problem hiding this comment.
Again, this file shouldn't exist
| @route("/chat") op chat(): AssistantMessage; | ||
| `, | ||
| [ | ||
| ["AssistantMessage.cs", [`public string Role { get; } = "assistant";`]], |
There was a problem hiding this comment.
I'd like to test for the rest of the defintion here (I think it should either be an Object or a generic json type
| ], | ||
| ], | ||
| ["WidgetList.cs", ["// Generated by @typespec/http-server-csharp", "using System;"]], | ||
| ["IContosoOperations.cs", ["Task<Widget[]> GetWidgetsAsync( )"]], |
There was a problem hiding this comment.
Here we should make sure there is an appropriate using for the namespace containing Widget
| arrayDeclarationContext(array: Model, name: string, elementType: Type) { | ||
| const arrayName = ensureCSharpIdentifier(this.emitter.getProgram(), array, name); | ||
| const arrayFile = this.emitter.createSourceFile(`generated/models/${arrayName}.cs`); | ||
| arrayFile.meta[this.#sourceTypeKey] = CSharpSourceType.Model; |
There was a problem hiding this comment.
We could potentially use this to not emit the useless array model. Ideally, we could create a context without an emitted file.
In my work in the openai-openapi-pr repo, the service emitter would not compile and gave the following error message:
I fixed the issue when I added
arrayDeclarationContextto the localnode_modulesin my repo. Here is the fix that I made as well as a test that demonstrates the issue.