Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[TCGC] client type for TypeSpec empty model #846

Closed
Tracked by #1356
tadelesh opened this issue May 16, 2024 · 7 comments
Closed
Tracked by #1356

[TCGC] client type for TypeSpec empty model #846

tadelesh opened this issue May 16, 2024 · 7 comments
Assignees
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area

Comments

@tadelesh
Copy link
Member

tadelesh commented May 16, 2024

In modeler4, we have anyObject type to represent an empty model definition despite whether it is a named definition or not (see the type of prop1 and prop2 for the following example). And most languages I believe will treat it as any type.

"definitions": {
  "EmptyModel": {
    "type": "object"
  },
  "Test": {
    "type": "object",
    "properties": {
      "prop1": {
        "type": "object"
      },
      "prop2": {
        "$ref": "#/definitions/EmptyModel"
      }
    },
    "required": [
      "prop1",
      "prop2"
    ]
  }
}

But in TypeSpec, we could clearly say a model is a named model without any properties or an anonymous model without any properties (type of prop1 and prop2).

model Test {
  prop1: {};
  prop2: EmptyModel;
}
model EmptyModel {}

So, we need to get consensus on what type we should generate for TypeSpec empty model, and also it should support non-breaking change for brown field services.

Proposal:
Introduce anyObject type in TCGC. Anonymous model without any properties will be converted to anyObject. For now, unknown is converted to any in TCGC.
Named model without any properties will be kept as a named model.
For brown field service, converter should always use anonymous model in TypeSpec without any properties to represent anyObject type in m4.

cc: @Azure/dpg-devs

@tadelesh
Copy link
Member Author

tadelesh commented May 20, 2024

Conclude all possible cases in the following table, example is in this playground:

Swagger m4 TypeSpec TCGC example
{"type": "object"} anyObject {} anonymous model with no properties prop1
{"type": "object"} anyObject model EmptyModel {} named model with no properties prop2
{} any unknown any prop3
{"type": "object", "additionalProperties": {}} dictionary with any type Record<unknown> dictionary with any type prop4
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties extends Record<unknown> {} named model with only additional properties prop5
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties is Record<unknown> {} named model with only additional properties prop6
{"type": "object", "additionalProperties": {}} dictionary with any type model EmptyModelWithAdditionalProperties {...Record<unknown>} named model with only additional properties prop7

@timotheeguerin could you help to confirm this is the right behavior for swagger/m4?
@Azure/dpg-devs: please confirm the behavior for TCGC.

Since swagger/m4 one expression could map several TypeSpec expression, for brown field service converting to TypeSpec, we will use the following mapping:

m4 TypeSpec
anyObject {}
any unknown
dictionary with any type Record<unknown>

@iscai-msft
Copy link
Contributor

Agree with the mapping at the end, we shouldn't do any extra conversion bc of the possibility of breaking changes

@pshao25
Copy link
Member

pshao25 commented May 30, 2024

Decision: everything keeps the same, only in conversion tool, change anyObject in M4 to {} in TypeSpec.

@tadelesh
Copy link
Member Author

tadelesh commented May 31, 2024

since typespec has more expression ability than swagger, we decided to use more accurate result when converting from swagger to typespec. if the conversion result is not the one service want, typespec author needs to change manually.
regarding how languges' emitter deal with the empty model is depends on languages' architect. tcgc will keep the full info and do no implicit conversion.

@XiaofeiCao
Copy link
Contributor

XiaofeiCao commented Jul 12, 2024

I want to confirm the difference between anyObject in M4 and named/anonymous model in TypeSpec.

In Swagger, {"type": "object"}(or anyObject) means Free-Form Object, which can have either arbitrary properties or none.
Screenshot 2024-07-12 at 10 07 28

So my questions are:

  1. If we add properties definition to a Free-Form Object in Swagger, is it a breaking change? I tend to believe it is, since it adds additional constraint to the model properties(something like adding a required property).
  2. Does TCGC consider named/anonymous model in TypeSpec both be represented as anyObject?
  3. Does TypeSpec consider adding properties to the empty model a breaking change?

@tadelesh
Copy link
Member Author

although in m4 we treat {"type": "object"} as anyObject, but we do not believe all the service use it in the right way. so, for swagger converter, it will be converted to anonymous model. if service team think the result is wrong, they need to change it manually. and in typespec, i believe any model without any property is just an empty model, if want any then use unknown, if want any object, then use Record<unknown>.

@XiaofeiCao
Copy link
Contributor

Thanks. My takeaways is,

  1. Empty model in TypeSpec means model without properties, not anyObject.
  2. For brownfields, by default we convert anyObject as TypeSpec (anonymous)empty model. If service really wants anyObject, TypeSpec's anyObject equivalent is Record<unknown>.

CMIIW.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
lib:tcgc Issues for @azure-tools/typespec-client-generator-core library needs-area
Projects
None yet
Development

No branches or pull requests

5 participants