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

Anonymous models inline support #2072

Merged
merged 36 commits into from
Oct 25, 2023

Conversation

MaryGao
Copy link
Contributor

@MaryGao MaryGao commented Oct 19, 2023

fixes #2067

Copy link
Member

@joheredi joheredi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, left a few minor comments. I'm really excited to see this working now 😄

@MaryGao MaryGao marked this pull request as ready for review October 25, 2023 06:47
});
});
describe("happens as a property in response body", async () => {
it("should map empty anonymous model({}) => Record<string, any> & empty named model(A {}) => Record<string, any>", async () => {
Copy link
Contributor Author

@MaryGao MaryGao Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call out for this test case

  • For the anonymous empty model {}, we take it as Record<string, any>. the benefit is the customers could assign objects with properties but we may take potential breaking risk if the contract is changed to e.g { a?: string }

  • For named empty model A {} I prefer to respect the name and structure in typespec, so the model A would be generated in models.ts and be referred in operations.ts. If customers add more optional properties in it would not cause any breakings.

/cc @joheredi Feel free to share your ideas.


it("should return non-empty anonymous model({ ... })", async () => {
const tspContent = `
op read(): { foo?: {bar: string | null}};
Copy link
Contributor Author

@MaryGao MaryGao Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please notice if the return type is anonymous the inline type would be duplicated in different places.

export async function _readDeserialize(
     result: Read200Response
): Promise<{ foo?: { bar: string | null } }>

export async function read(
    context: Client,
    options: ReadOptions = { requestOptions: {} }
): Promise<{ foo?: { bar: string | null } }>

Copy link
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if you have added test for the sample generation with anonymous model ?

@MaryGao
Copy link
Contributor Author

MaryGao commented Oct 25, 2023

wonder if you have added test for the sample generation with anonymous model ?

Added here abec42a

@MaryGao MaryGao merged commit d913c84 into Azure:main Oct 25, 2023
28 checks passed
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.

Dpg - Generate anonymous models inline
3 participants