Upgrade compiler v0.48 #2029

MaryGao · 2023-09-19T09:50:46Z

Upgrade compiler v0.48

MaryGao · 2023-09-19T09:52:36Z

packages/typespec-ts/src/modular/buildCodeModel.ts

@@ -1246,23 +1246,23 @@ function emitUnion(context: SdkContext, type: Union): Record<string, any> {
      description: `Type of ${unionName}`,
      internal: true,
      type: "combined",
-      types: sdkType.values.map((x) => getType(context, x.__raw)),
+      types: sdkType.values.map((x) => getType(context, x.__raw!)),


Confirming with isa: https://github.com/Azure/typespec-azure/pull/3547#discussion_r1329735040

packages/typespec-ts/src/modular/buildCodeModel.ts

MaryGao · 2023-09-19T12:17:12Z

packages/typespec-ts/src/modular/buildCodeModel.ts

@@ -1644,7 +1629,7 @@ export function emitCodeModel(
  simpleTypesMap.clear();
  const allModels = getAllModels(dpgContext);


@iscai-msft @tadelesh After upgrading to the latest tcgc our inheritance test cases failed. I checked the getAllModels interface would only return base model Pet to us, in the old version it would return 3 models(1 base model + 2 derived models).

I believe getAllModels should return both base and derived models, Could you check if there is a bug in tcgc?

autorest.typescript/packages/typespec-ts/test/modularUnit/modelsGenerator.spec.ts

Lines 771 to 805 in d94c5fd

it("should handle inheritance model", async () => {

const modelFile = await emitModularModelsFromTypeSpec(`

model Pet {

name: string;

weight?: float32;

}

model Cat extends Pet {

kind: "cat";

meow: int32;

}

model Dog extends Pet {

kind: "dog";

bark: string;

}

op read(): { @body body: Pet };

`);

assert.ok(modelFile);

assertEqualContent(

modelFile?.getFullText()!,

`

export interface Pet {

name: string;

weight?: number;

}

export interface Cat extends Pet {

kind: "cat";

meow: number;

}

export interface Dog extends Pet {

kind: "dog";

bark: string;

}`

);

Since Pet is not a discriminated type, Dog and Cat will not be used in read, so getAllModel will not return these two orphan models.

Thanks for the info! I am checking the semantic with TypeSpec team: microsoft/typespec#2450.

MaryGao · 2023-09-19T13:27:40Z

packages/typespec-ts/test/modularUnit/type.spec.ts

      export interface Test {
-        color: "red" | "blue";
+        color: ColorType;


@iscai-msft @tadelesh Another failed test case is for union of string.

TypeSpec

model Test { color: "red" | "blue"; }

Old generation after calling getSdkUnion

/** Type of ColorType */ export type ColorType = "red" | "blue"; export interface Test { color: ColorType; }

Latest generation with getSdkUnion

export interface Test { color: "red"; }

After debugging I found the getSdkUnion would only return the first enum member to us. Here is the relevant tcgc code.

talked with the dpg crew about this, for sdk union types like this scenario, where the definition is `, we've decided to just emit it as a string. This is because there isn't information about the name of the union, and dotnet and java can't handle this. If they want an enum, they should define it as an enum in the tsp file, instead of us creating an enum from it

That's interesting! I think emitting this as a string is not proper for TypeScript because we have native support for string union. And for TypeSpec type "red" | "blue", we may prefer to directly map to TypeScript type "red" | "blue" like below. But currently we only have the first enum in underlying __raw, it makes us impossible to get all values.

On the other hand for other languages they emit as a string but they may have pontential requirement to touch underlying values, e.g using them in descriptions or knownable values etc.

export interface Test { color: "red" | "blue"; }

@joheredi Have you been involved in those discussions and are you agree with this ?

IIRC we agreed to have it as a string in tcgc since 3/4 languages are just going to emit it as a string, which conflicts with what JS wants here. For JS though, I should be able to fix the __raw to return the original union type and there should be enough information there. Down the line, we might decide that we can toggle this behavior with a flag, but since I think it's very JS specific, I think the best way right now is to have JS get the info they need themselves from the raw type

Version 0.35.0-dev.5 is currently releasing and has this change

Thank you for the quick fix! Not sure we could have a stable patch version for this?

.vscode/launch.json

MaryGao · 2023-09-22T05:25:07Z

packages/typespec-test/test/loadTest/spec/routes.tsp

@@ -298,7 +298,7 @@ numeric, underscore or hyphen characters.

  @summary("Create and start a new test run with the given name.")
  @doc("Create and start a new test run with the given name.")
-  @pollingOperation(LoadTestRun.GetTestRun)
+  // @pollingOperation(LoadTestRun.GetTestRun)


Prefer to disable this customized pollingOperation because it is not aligned with latest lro template.

https://github.com/Azure/typespec-azure/issues/3619#issuecomment-1729597022

MaryGao · 2023-09-22T05:25:59Z

packages/typespec-ts/test/modularUnit/type.spec.ts

+    });
+
+    // TODO: tgcg lib has a bug here: https://github.com/Azure/typespec-azure/issues/3623
+    it.skip("nullable string literal", async () => {


Skip this testing due to https://github.com/Azure/typespec-azure/issues/3623

MaryGao · 2023-09-22T07:25:03Z

packages/typespec-test/test/openai_modular/generated/typespec-ts/src/rest/responses.ts

@@ -100,13 +100,6 @@ export interface GetAzureBatchImageGenerationOperationStatusDefaultResponse
    GetAzureBatchImageGenerationOperationStatusDefaultHeaders;
 }

-/** The final response for long-running getAzureBatchImageGenerationOperationStatus operation */
-export interface GetAzureBatchImageGenerationOperationStatusLogicalResponse


The get polling operation won't be taken as lro in latest getLroMetadata, so relevant logic response will be removed.

So I think this change is expected. Confirmed with .Net and Java they automatically filter out this polling method so it would not impact them.

MaryGao · 2023-09-22T07:25:54Z

packages/typespec-ts/src/modular/buildCodeModel.ts

@@ -171,15 +173,6 @@ function getDocStr(program: Program, target: Type): string {
  return getDoc(program, target) ?? "";
 }

-function isLro(_program: Program, operation: Operation): boolean {


No need to build modular lro helper because we can leverage existing one.

MaryGao · 2023-09-22T07:47:37Z

packages/typespec-test/test/loadTest/generated/openapi/2022-11-01/openapi.json

@@ -3097,6 +3096,50 @@
        }
      }
    },
+    "TestRunCreateOrUpdate": {


The change is introduced by recent change: microsoft/typespec#2291

.vscode/launch.json

MaryGao · 2023-09-22T08:01:14Z

common/config/rush/pnpm-config.json

+  "useWorkspaces": true,
+  "strictPeerDependencies": false,
+  "globalOverrides": {
+    "@azure-tools/typespec-client-generator-core": "0.35.0-dev.5"


Pending tcgc release

MaryGao · 2023-09-22T08:12:54Z

Still one paging detail to confirm with Tim but may not block this pr: https://github.com/Azure/typespec-azure/pull/3382#discussion_r1332552725.

qiaozha · 2023-09-25T03:43:23Z

packages/typespec-test/test/confidentialLedger/generated/openapi/openapi.json

@@ -672,6 +672,16 @@
        "assignedRole"
      ]
    },
+    "LedgerUserCreateOrUpdate": {


What is the root cause of the changing from LedgerUserUpdate to LedgerUserCreateOrUpdate ?

see comment: #2029 (comment), so they change the visibility for patch method from update to both update and create methods so relevant model name changed.

qiaozha · 2023-09-25T03:44:38Z

...ges/typespec-test/test/contoso/generated/typespec-ts/review/contosowidgetmanager-rest.api.md

@@ -242,7 +231,7 @@ export function isUnexpected(response: CreateOrUpdateWidget200Response | CreateO
 export function isUnexpected(response: DeleteWidget202Response | DeleteWidgetLogicalResponse | DeleteWidgetDefaultResponse): response is DeleteWidgetDefaultResponse;

 // @public (undocumented)
-export function isUnexpected(response: GetWidgetOperationStatus200Response | GetWidgetOperationStatusLogicalResponse | GetWidgetOperationStatusDefaultResponse): response is GetWidgetOperationStatusDefaultResponse;
+export function isUnexpected(response: GetWidgetOperationStatus200Response | GetWidgetOperationStatusDefaultResponse): response is GetWidgetOperationStatusDefaultResponse;


why missing the logic response here ?

see comment: #2029 (comment)

qiaozha · 2023-09-25T03:48:20Z

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

@@ -38,13 +35,13 @@ export interface ListWidgetsOptions extends OperationOptions {

 // @public (undocumented)
 export interface UpdateWidgetOptions extends OperationOptions {
-    color?: ColorType;


Can we keep the colorType ? Is it from typepsec or codegen made it up ?

The ColorType is made by TCGC, now they prefer to model it as string. See comments #2029 (comment).

Personally I prefer not to give a name for it. Because we don't know the name and it may conflict with customer's own.

qiaozha · 2023-09-25T03:55:22Z

packages/typespec-ts/src/utils/clientUtils.ts

@@ -10,11 +10,18 @@ export function getRLCClients(dpgContext: SdkContext): SdkClient[] {
      kind: "SdkClient",
      name: service.type.name + "Client",
      service: service.type,
-      type: service.type
+      type: service.type,
+      arm: isArm(service.type)


why add this ? do we have any current different behaviors between ARM and non ARM in RLC ?

In the latest tcgc the arm becomes a required property and I copied the logic in tcgc to apply its value, but it is not used now.

qiaozha · 2023-09-25T03:57:46Z

packages/typespec-ts/test/unit/modelsGenerator.spec.ts

-      @@projectedName(FooModel.x, "json", "xJson")
+      @@projectedName(FooModel.x, "client", "NotToUseMeAsName"); // Should be ignored
+      @@projectedName(FooModel.x, "javascript", "MadeForTS");
+      @@projectedName(FooModel.x, "json", "xJson");


Is it okay to add ; at the end of decorator ?

this is new breaking https://microsoft.github.io/typespec/release-notes/release-2023-09-12#typespeccompiler-breaking-changes

qiaozha · 2023-09-25T05:10:45Z

packages/typespec-ts/src/modular/buildCodeModel.ts

-    format: sdkType.format,
-    ...extraInformation
+    doc: "",
+    apiVersions: [],


Is the apiVersions info also getting removed in tcgc ? I think we need those when we are working on multi api version ?

This is suggested by Isa, and it is only used to set the default value for simple model like string so I think it's okay because we don't need to detect the version change for simple type.

packages/typespec-ts/test/modularUnit/modelsGenerator.spec.ts

packages/typespec-ts/src/modular/buildCodeModel.ts

Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com>

qiaozha

LGTM

packages/typespec-ts/test/modularUnit/modelsGenerator.spec.ts

packages/typespec-ts/src/modular/buildCodeModel.ts

* upgrade the tsp compiler version * Update the breakings * Fix to pass build errors * Update the ! * Fix the ut issue in rlc * Update the test cases * Update the test cases * update the lib to dev * Update the test cases * Update the spec * Update the lint * Disable the polling operation setting due to wrong def * Update .vscode/launch.json * Update .vscode/launch.json * Update the error spec * temp * temp * update the test cases * Update the test cases * update changes in openai * update the statement * Update the modular lro * Update the openapi.json * Update .vscode/launch.json * Update packages/typespec-ts/src/modular/buildCodeModel.ts Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com> * update the name * Update the dependency to dev --------- Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com>

MaryGao added 5 commits September 19, 2023 15:20

upgrade the tsp compiler version

b2bf37f

Update the breakings

d6cb382

Merge remote-tracking branch 'origin/main' into upgrade-compiler-v0.48

f7b17e8

Fix to pass build errors

eafebb6

Update the !

77b8a5f

MaryGao commented Sep 19, 2023

View reviewed changes

packages/typespec-ts/src/modular/buildCodeModel.ts Show resolved Hide resolved

Fix the ut issue in rlc

d94c5fd

MaryGao commented Sep 19, 2023

View reviewed changes

MaryGao added 2 commits September 19, 2023 21:21

Update the test cases

f7a65f3

Update the test cases

93d6d06

MaryGao commented Sep 19, 2023

View reviewed changes

MaryGao added 5 commits September 21, 2023 10:29

update the lib to dev

c0b6f13

Update the test cases

afdb56f

Update the spec

b258ae1

Update the lint

4db33a8

Disable the polling operation setting due to wrong def

748745b

MaryGao commented Sep 21, 2023

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

Update .vscode/launch.json

d700808

MaryGao commented Sep 21, 2023

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

MaryGao added 6 commits September 21, 2023 21:12

Update .vscode/launch.json

3d4fe46

Update the error spec

9beaf7a

temp

bece042

temp

cd9ee55

update the test cases

2b4b536

Update the test cases

0bd3065

MaryGao commented Sep 22, 2023

View reviewed changes

MaryGao added 2 commits September 22, 2023 14:18

update changes in openai

a519249

update the statement

35d1d4d

Update the modular lro

f8f749f

MaryGao commented Sep 22, 2023

View reviewed changes

Update the openapi.json

d2d7d3f

MaryGao commented Sep 22, 2023

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

Update .vscode/launch.json

6d83594

MaryGao commented Sep 22, 2023

View reviewed changes

MaryGao marked this pull request as ready for review September 22, 2023 08:40

MaryGao requested review from sarangan12 and joheredi as code owners September 22, 2023 08:40

qiaozha reviewed Sep 25, 2023

View reviewed changes

MaryGao and others added 2 commits September 25, 2023 17:54

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

a803437

Co-authored-by: Qiaoqiao Zhang <55688292+qiaozha@users.noreply.github.com>

update the name

11fbfdd

qiaozha approved these changes Sep 26, 2023

View reviewed changes

packages/typespec-ts/test/modularUnit/modelsGenerator.spec.ts Show resolved Hide resolved

packages/typespec-ts/src/modular/buildCodeModel.ts Show resolved Hide resolved

Update the dependency to dev

53f4ccc

MaryGao merged commit 674f60d into Azure:main Sep 26, 2023
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade compiler v0.48 #2029

Upgrade compiler v0.48 #2029

MaryGao commented Sep 19, 2023 •

edited

Loading

MaryGao Sep 19, 2023

MaryGao Sep 19, 2023 •

edited

Loading

tadelesh Sep 20, 2023

MaryGao Sep 20, 2023

MaryGao Sep 19, 2023 •

edited

Loading

iscai-msft Sep 19, 2023

MaryGao Sep 20, 2023 •

edited

Loading

qiaozha Sep 20, 2023

iscai-msft Sep 20, 2023

iscai-msft Sep 20, 2023 •

edited

Loading

MaryGao Sep 21, 2023

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao Sep 22, 2023

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao Sep 22, 2023

MaryGao Sep 22, 2023

MaryGao commented Sep 22, 2023 •

edited

Loading

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023 •

edited

Loading

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023 •

edited

Loading

qiaozha Sep 25, 2023

MaryGao Sep 25, 2023 •

edited

Loading

qiaozha left a comment

		@@ -1644,7 +1629,7 @@ export function emitCodeModel(
		simpleTypesMap.clear();
		const allModels = getAllModels(dpgContext);

	it("should handle inheritance model", async () => {
	const modelFile = await emitModularModelsFromTypeSpec(`
	model Pet {
	name: string;
	weight?: float32;
	}
	model Cat extends Pet {
	kind: "cat";
	meow: int32;
	}
	model Dog extends Pet {
	kind: "dog";
	bark: string;
	}
	op read(): { @body body: Pet };
	`);
	assert.ok(modelFile);
	assertEqualContent(
	modelFile?.getFullText()!,
	`
	export interface Pet {
	name: string;
	weight?: number;
	}

	export interface Cat extends Pet {
	kind: "cat";
	meow: number;
	}

	export interface Dog extends Pet {
	kind: "dog";
	bark: string;
	}`
	);

Upgrade compiler v0.48 #2029

Upgrade compiler v0.48 #2029

Conversation

MaryGao commented Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

MaryGao Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iscai-msft Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

MaryGao Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao commented Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

MaryGao Sep 25, 2023 • edited Loading

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 Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryGao Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

qiaozha left a comment

Choose a reason for hiding this comment

MaryGao commented Sep 19, 2023 •

edited

Loading

MaryGao Sep 19, 2023 •

edited

Loading

MaryGao Sep 19, 2023 •

edited

Loading

MaryGao Sep 20, 2023 •

edited

Loading

iscai-msft Sep 20, 2023 •

edited

Loading

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao Sep 22, 2023 •

edited

Loading

MaryGao commented Sep 22, 2023 •

edited

Loading

MaryGao Sep 25, 2023 •

edited

Loading

MaryGao Sep 25, 2023 •

edited

Loading

MaryGao Sep 25, 2023 •

edited

Loading