Binder models tcgc #2759

joheredi · 2024-08-19T18:33:08Z

Summary of Changes in this PR

use binder to manage model references
deserializer support. fixes Fill in gaps for Deserialization #2625
refactor of the operation helper in both serialization and deserialization. partially fix Serialization Followup work #2619 combine the previous serializeUtils/deserializer logic with serializer/deserializer.
sdkPackage.models related adoption. Implement TCGC's SdkPackage types #2406
client initialization adoption fixes Support client-level parameter defined @clientInitialization #2795
use tcgc generated name for anonymous model
rename RLC context name with tcgc client name.
stop normalize the property name Should we cast property name automatically in modular? #2645
refactor of all unit test to take options parameter instead of introducing long signature list.
modular models for multiclient support. fixes revisit modular models generation in multi clients #2611
a. generate one copy of models outside of the subclient folder of multi-client case.
b. move operation options inside the api folder.

TCGC related issues

packages/typespec-ts/src/index.ts

...spec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/api/operations/index.ts

...es/typespec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/models/models.ts

packages/typespec-ts/src/modular/emit-models.ts

...s/typespec-test/test/anomalyDetector/generated/typespec-ts/review/ai-anomaly-detector.api.md

qiaozha · 2024-09-05T02:11:47Z

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md

+}
+
+// @public (undocumented)
+export interface DeleteDataRequest {


this seems to be a new model. not sure where it comes from

so is this expected?

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md

...ec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/api/dataProducts/index.ts

packages/typespec-ts/src/index.ts

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md

packages/typespec-test/test/eventgrid_modular/generated/typespec-ts/src/api/eventGridContext.ts

...pespec-test/test/contentsafety_modular/generated/typespec-ts/review/ai-content-safety.api.md

...pespec-test/test/contentsafety_modular/generated/typespec-ts/src/api/contentSafetyContext.ts

...pespec-test/test/contentsafety_modular/generated/typespec-ts/review/ai-content-safety.api.md

...ec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/api/dataProducts/index.ts

qiaozha · 2024-09-18T03:39:21Z

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md

@@ -439,6 +455,10 @@ export interface NetworkAnalyticsClientOptionalParams extends ClientOptions {
    apiVersion?: string;
 }

+// @public (undocumented)


docs missing.

...es/typespec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/models/models.ts

packages/typespec-ts/test/modularUnit/scenarios/models/deserialization/propertyType.md

packages/typespec-test/test/openai_modular/generated/typespec-ts/review/openai_modular.api.md

packages/typespec-test/test/openai_modular/generated/typespec-ts/src/index.ts

timovv · 2024-10-10T21:28:53Z

packages/typespec-test/test/openai_modular/generated/typespec-ts/src/models/models.ts

+    timestamp_granularities: !item["timestampGranularities"]
+      ? item["timestampGranularities"]
+      : item["timestampGranularities"].map((p: any) => {
+          return p;
+        }),


Feels like this is a no-op? Could we just generate timestamp_granularities: item["timestampGranularities"] or am I missing something?

yeah, but I was actually struggling between the option of increasing the codegen implementation/runtime complexity by detecting if we need to generate the serializer/deserializer for each type recursively with the risk of some type we may be missing, and the option of just generate the serializer/deserializer any way.

timovv · 2024-10-10T21:41:53Z

packages/typespec-test/test/openai_modular/generated/typespec-ts/src/models/models.ts

+    functions: !item["functions"]
+      ? item["functions"]
+      : functionDefinitionArraySerializer(item["functions"]),


Would it work to simplify this pattern to something like:

{ // ... functions: item["functions"] && functionDefinitionArraySerializer(item["functions"]), }

or is there a case where that wouldn't do the right thing?

timovv · 2024-10-10T22:04:44Z

packages/typespec-test/test/openai_modular/generated/typespec-ts/review/openai_modular.api.md

@@ -990,9 +1034,6 @@ export interface PineconeFieldMappingOptions {
    urlField?: string;
 }

-// @public


Is it expected that this is being removed?

I think this should be expected.
currently, we will filter the generated types based on their usage here https://github.com/joheredi/autorest.typescript/blob/0d4a12d15dbcfdff103e85804b0fc5fe23c512d8/packages/typespec-ts/src/modular/emitModels.ts#L100-L107 , And in tcgc, this version enum has a special UsageFlag https://github.com/Azure/typespec-azure/blob/main/packages/typespec-client-generator-core/src/interfaces.ts#L703. that's why it gets filtered out.

We have an issue for version union #2680, which we have agreed to generate the api version type as string, but we will generate a KnownXXX enum for it as document purpose.

Because issue #2680 is not completed yet, and if I remember correctly, in our existing releases, @kazrael2119 has manually removed all the version unions when preparing the prs. so, it should not be a breaking.

@dgetu I think you have the same comment about this. Let me know if you have different thoughts here.

packages/typespec-ts/src/modular/helpers/clientHelpers.ts

packages/typespec-ts/src/modular/serialization/buildDeserializerFunction.ts

packages/typespec-ts/src/modular/serialization/buildSerializerFunction.ts

dgetu · 2024-10-11T00:56:11Z

packages/typespec-ts/test/modularIntegration/generated/azure/core/lro/standard/src/index.d.ts

-    createOrReplace(name: string, resource: User, options?: CreateOrReplaceOptionalParams): PollerLike<OperationState<User>, User>;
-    delete(name: string, options?: DeleteOptionalParams): PollerLike<OperationState<void>, void>;
-    export(name: string, format: string, options?: ExportOptionalParams): PollerLike<OperationState<ExportedUser>, ExportedUser>;
+    createOrReplace(name: string, resource: User, options?: CreateOrReplaceOptionalParams): PollerLike<OperationState_2<User>, User>;


blocking: Naming is broken here

dgetu · 2024-10-11T00:58:22Z

...-ts/test/modularIntegration/generated/azure/resource-manager/models/resources/src/index.d.ts

@@ -94,6 +110,10 @@ export declare interface PageSettings {

 export declare type ProvisioningState = ResourceProvisioningState | "Provisioning" | "Updating" | "Deleting" | "Accepted";

+export declare type ProvisioningState_1 = ResourceProvisioningState | "Provisioning" | "Updating" | "Deleting" | "Accepted";


check: Type is being generated multiple times. Is this a namespace problem, or an emitter bug?

...-ts/test/modularIntegration/generated/client/structure/client-operation-group/src/index.d.ts

packages/typespec-ts/test/modularIntegration/generated/client/structure/default/src/index.d.ts

...spec-ts/test/modularIntegration/generated/type/property/additional-properties/src/index.d.ts

Co-authored-by: Daniel Getu <dgetu00@gmail.com>

MaryGao · 2024-10-14T02:24:55Z

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md

@@ -313,6 +318,10 @@ export interface ErrorAdditionalInfo {
    readonly type?: string;
 }

+// @public
+export interface ErrorAdditionalInfoInfo {


new orphan model?

MaryGao · 2024-10-14T02:27:45Z

packages/typespec-test/test/NetworkAnalytics.Management/generated/typespec-ts/src/index.ts

-  Versions,
+} from "./models/index.js";
+export {
+  NetworkAnalyticsContext,


why we export NetworkAnalyticsContext to public in classical layer? I think this is implementation detail in this layer.

MaryGao · 2024-10-14T04:12:58Z

packages/typespec-test/test/eventgrid_modular/generated/typespec-ts/review/eventgrid.api.md

@@ -34,7 +35,7 @@ export interface BrokerProperties {
 // @public
 export interface CloudEvent {
    data?: any;
-    dataBase64?: Uint8Array;
+    data_base64?: Uint8Array;


I am a fan of that we should do property normalization especially for azure scope. This could help us generate more idiomatic SDKs. And could ensure consistant property naming style cross SDKs.

TypeSpec authors could have their own style of property naming convention and that style may or may not be suitable for JS/TS language style. So property normalization could help

ensure all properties only have one style

property naming style is JS/TS style

good property name could be better discoverable by user intellisense

a typespec property may not be a valid ts property

joheredi added 2 commits August 15, 2024 16:16

Generate models from TCGC and use binder

10267e1

generic serializers

1d0e542

qiaozha reviewed Aug 20, 2024

View reviewed changes

packages/typespec-ts/src/index.ts Outdated Show resolved Hide resolved

qiaozha self-assigned this Aug 28, 2024

qiaozha added 4 commits August 29, 2024 15:09

fix const and enum value expression

5522764

merge main and resolve conflicts

ff42613

merge main

fb5cc9f

fix paged result model name

136a66b

qiaozha reviewed Sep 3, 2024

View reviewed changes

qiaozha and others added 4 commits September 3, 2024 15:34

Update packages/typespec-ts/src/modular/emit-models.ts

1205b52

remove old imports and refactor serializer util

e9446b3

fix serializer

efe343b

fix anomalydetector

1f251c3

qiaozha reviewed Sep 4, 2024

View reviewed changes

...s/typespec-test/test/anomalyDetector/generated/typespec-ts/review/ai-anomaly-detector.api.md Outdated Show resolved Hide resolved

fix readonly, enum docs missing, serializeRecord crash issues

e8ffadb

qiaozha reviewed Sep 5, 2024

View reviewed changes

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md Outdated Show resolved Hide resolved

qiaozha reviewed Sep 5, 2024

View reviewed changes

qiaozha and others added 6 commits September 5, 2024 10:21

Update packages/typespec-ts/src/index.ts

6a7e682

Merge branch 'main' into binder-models-tcgc

cb143cc

fix extensible enum docs, knownvalue enum

be80522

fix known enum name

91e4e9e

Merge branch 'main' into binder-models-tcgc

c61258e

use binder to resolve dependencies and reserve work

92c5a25

qiaozha reviewed Sep 5, 2024

View reviewed changes

...st/test/NetworkAnalytics.Management/generated/typespec-ts/review/arm-networkanalytics.api.md Outdated Show resolved Hide resolved

qiaozha reviewed Sep 5, 2024

View reviewed changes

joheredi added 2 commits September 5, 2024 15:22

Fix missing TokenCredential imports

5149443

Fix client not found issue

53e235f

joheredi force-pushed the binder-models-tcgc branch from 92c5a25 to 1d0e542 Compare September 5, 2024 22:57

fix client factory signature

a6384c7

qiaozha requested a review from dgetu September 30, 2024 07:34

qiaozha added 3 commits September 30, 2024 22:31

fix ci

b6a60ee

add unit test

3444ad7

add extensible enum case

286f8c9

qiaozha reviewed Oct 8, 2024

View reviewed changes

qiaozha and others added 10 commits October 9, 2024 10:46

add additional properties ser/deser unit test

9a87639

fix extensible enum inline generation

b21b83d

fix ci

6257aaf

Merge branch 'main' into binder-models-tcgc

27bcd94

fix duplicate expression

9fa085f

format change

5ad149e

fix unit test

653a022

fix unused identifiers

898f6f9

fix format and ci

78eff56

Merge branch 'main' into binder-models-tcgc

6e179c1

timovv reviewed Oct 10, 2024

View reviewed changes

dgetu reviewed Oct 11, 2024

View reviewed changes

qiaozha and others added 9 commits October 11, 2024 13:17

remove api layer at top level index

3640607

resolve some comments

8c965c8

resolve comments

0d4a12d

Merge branch 'main' into binder-models-tcgc

d9f63a3

fix client parameter normalize

6c10613

merge main

cbdd7b4

fix discriminated union and polymorphic base

ab4991c

fix discriminated base in model properties

3fe29d2

Update packages/typespec-ts/src/modular/emitSamples.ts

5c34b3f

Co-authored-by: Daniel Getu <dgetu00@gmail.com>

MaryGao reviewed Oct 14, 2024

View reviewed changes

reserve work

b850fae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Binder models tcgc #2759

Binder models tcgc #2759

joheredi commented Aug 19, 2024 •

edited by qiaozha

Loading

qiaozha Sep 5, 2024

MaryGao Oct 14, 2024

qiaozha Sep 18, 2024

timovv Oct 10, 2024

qiaozha Oct 11, 2024

timovv Oct 10, 2024

timovv Oct 10, 2024

qiaozha Oct 11, 2024

qiaozha Oct 11, 2024

dgetu Oct 11, 2024

dgetu Oct 11, 2024

MaryGao Oct 14, 2024

MaryGao Oct 14, 2024

MaryGao Oct 14, 2024 •

edited

Loading

		@@ -94,6 +110,10 @@ export declare interface PageSettings {

		export declare type ProvisioningState = ResourceProvisioningState \| "Provisioning" \| "Updating" \| "Deleting" \| "Accepted";

		export declare type ProvisioningState_1 = ResourceProvisioningState \| "Provisioning" \| "Updating" \| "Deleting" \| "Accepted";

Binder models tcgc #2759

Are you sure you want to change the base?

Binder models tcgc #2759

Conversation

joheredi commented Aug 19, 2024 • edited by qiaozha Loading

Summary of Changes in this PR

TCGC related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

joheredi commented Aug 19, 2024 •

edited by qiaozha

Loading

MaryGao Oct 14, 2024 •

edited

Loading