Skip to content

Commit

Permalink
Fix compiler errors in tests and samples (#31649)
Browse files Browse the repository at this point in the history
After moving to ESM/tshy, we no longer build tests/samples. This PR
enables running "typecheck" as an optional rush bulk command, and fixes
some of the errors found.

The known errors reported on files from vite/vitest/chai are ignored.

The update-snippets dev-tool command throws error of reading undefined
in some cases, adding a null check fixes it.

Snippets are updated as well after snippets.spec.ts files are fixed.
  • Loading branch information
jeremymeng authored Nov 6, 2024
1 parent 36f4f4d commit cf8d25f
Show file tree
Hide file tree
Showing 31 changed files with 200 additions and 85 deletions.
14 changes: 14 additions & 0 deletions common/config/rush/command-line.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,20 @@
"enableParallelism": true,
"ignoreMissingScript": true
},
{
"commandKind": "bulk",
"name": "typecheck",
"summary": "type check files that are not part of the build",
"enableParallelism": true,
"ignoreMissingScript": true
},
{
"commandKind": "bulk",
"name": "update-snippets",
"summary": "Replace snippets placeholders with code extracted from TypeScript files",
"enableParallelism": true,
"ignoreMissingScript": true
},
{
"commandKind": "bulk",
"name": "lint:fix",
Expand Down
16 changes: 15 additions & 1 deletion common/tools/dev-tool/src/commands/run/typecheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export default leafCommand(commandInfo, async (options) => {

const project = new Project({
tsConfigFilePath: path.join(projPath, "tsconfig.json"),
compilerOptions: {
noUnusedLocals: false,
noUnusedParameters: false,
},
});

if (options.paths) {
Expand All @@ -39,7 +43,17 @@ export default leafCommand(commandInfo, async (options) => {
}
}

const diagnostics = project.getPreEmitDiagnostics();
const diagnostics = project.getPreEmitDiagnostics().filter((d) =>
{
const filepath = d.getSourceFile()?.getFilePath();
return !(
filepath?.includes("node_modules") &&
(filepath?.includes("@vitest") ||
filepath?.includes("vite-node") ||
filepath?.includes("chai"))
);
},
);
const hasError = diagnostics.some((d) => d.getCategory() === DiagnosticCategory.Error);
if (hasError) {
log.error(
Expand Down
5 changes: 4 additions & 1 deletion common/tools/dev-tool/src/commands/run/update-snippets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,10 @@ async function parseSnippetDefinitions(
// Get all the decls that are in source files and where the decl comes from an import clause.
return sym?.declarations
?.filter(
(decl) => decl.getSourceFile() === sourceFile && ts.isImportClause(decl.parent.parent),
(decl) =>
decl.getSourceFile() === sourceFile &&
decl.parent?.parent &&
ts.isImportClause(decl.parent.parent),
)
.map(
// It is a grammar error for moduleSpecifier to be anything other than a string literal. In future versions of
Expand Down
1 change: 1 addition & 0 deletions common/tools/dev-tool/src/util/resolveProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ declare global {
[k: string]: string[];
};
};
tshy?: Record<string, object>;
type?: string;
module?: string;
bin?: Record<string, string>;
Expand Down
1 change: 0 additions & 1 deletion sdk/batch/batch-rest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
"devDependencies": {
"@azure-tools/test-credential": "^1.0.0",
"@azure-tools/test-recorder": "^4.0.0",
"@azure-tools/test-utils": "~1.0.0",
"@azure-tools/vite-plugin-browser-test-map": "~1.0.0",
"@azure/core-util": "^1.0.0",
"@azure/dev-tool": "^1.0.0",
Expand Down
4 changes: 2 additions & 2 deletions sdk/batch/batch-rest/test/utils/recordedClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
// AzureCliCredential,
InteractiveBrowserCredential,
} from "@azure/identity";
import { isNode } from "@azure-tools/test-utils";
import { isNodeLike } from "@azure/core-util";
import { NoOpCredential } from "@azure-tools/test-credential";
import { AzureNamedKeyCredential } from "@azure/core-auth";

Expand Down Expand Up @@ -69,7 +69,7 @@ export async function createRecorder(ctx: VitestTestContext): Promise<Recorder>
export function createBatchClient(recorder?: Recorder, options: ClientOptions = {}): BatchClient {
const credential = isPlaybackMode()
? new NoOpCredential()
: isNode
: isNodeLike
? new AzureNamedKeyCredential(env.AZURE_BATCH_ACCOUNT!, env.AZURE_BATCH_ACCESS_KEY!)
: // : new AzureCliCredential();
new InteractiveBrowserCredential({
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/abort-controller/src/AbortError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* try {
* doAsyncWork({ abortSignal: controller.signal });
* } catch (e) {
* if (e.name === "AbortError") {
* if (e instanceof Error && e.name === "AbortError") {
* // handle abort error here.
* }
* }
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/abort-controller/test/snippets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe("snippets", () => {
try {
doAsyncWork({ abortSignal: controller.signal });
} catch (e) {
if (e.name === "AbortError") {
if (e instanceof Error && e.name === "AbortError") {
// handle abort error here.
}
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-client-rest/src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ export interface Client {
* strong types. When used by the codegen this type gets overridden with the generated
* types. For example:
* ```typescript snippet:path_example
* import { Client, Routes } from "@azure-rest/core-client";
* import { Client } from "@azure-rest/core-client";
*
* export type MyClient = Client & {
* type MyClient = Client & {
* path: Routes;
* };
* ```
Expand Down
13 changes: 11 additions & 2 deletions sdk/core/core-client-rest/test/snippets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,20 @@
// Licensed under the MIT License.

import { describe, it } from "vitest";
import type { Client, Routes } from "@azure-rest/core-client";
import type { Client } from "@azure-rest/core-client";

interface GetOperationResult {}
interface DetectFromUrl {}
interface Routes {
/** Resource for '/operations/\{operationId\}' has methods for the following verbs: get */
(path: "/operations/{operationId}", operationId: string): GetOperationResult;
/** Resource for '/detect' has methods for the following verbs: post */
(path: "/detect"): DetectFromUrl;
}

describe("snippets", () => {
it("path_example", () => {
export type MyClient = Client & {
type MyClient = Client & {
path: Routes;
};
});
Expand Down
1 change: 0 additions & 1 deletion sdk/core/core-client/test/snippets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { describe, it } from "vitest";

describe("snippets", () => {
it("authorize_request_on_claim_challenge", () => {
// @ts-ignore
const policy = bearerTokenAuthenticationPolicy({
challengeCallbacks: {
authorizeRequestOnChallenge: authorizeRequestOnClaimChallenge,
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-lro/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ A poller is an object that can poll the long running operation on the server for
A type for the operation state. It contains a `status` field with the following possible values: `notStarted`, `running`, `succeeded`, `failed`, and `canceled`. It can be accessed as follows:

```typescript snippet:operation_state
switch (poller.getOperationState().status) {
switch (poller.operationState.status) {
case "succeeded": // return poller.getResult();
case "failed": // throw poller.getOperationState().error;
case "canceled": // throw new Error("Operation was canceled");
Expand Down
4 changes: 3 additions & 1 deletion sdk/core/core-lro/test/snippets.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { PollerLike } from "@azure/core-lro";
import { describe, it } from "vitest";

const poller = {} as unknown as PollerLike<any, any>;
describe("snippets", () => {
it("operation_state", async () => {
switch (poller.getOperationState().status) {
switch (poller.operationState.status) {
case "succeeded": // return poller.getResult();
case "failed": // throw poller.getOperationState().error;
case "canceled": // throw new Error("Operation was canceled");
Expand Down
6 changes: 4 additions & 2 deletions sdk/core/core-paging/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,20 @@ You can find an explanation of how this repository's code works by going to our
Example of building with the types:

```typescript snippet:paging_example
import { PagedAsyncIterableIterator, PageSettings } from "@azure/core-paging";

function listSecrets(
options: ListSecretsOptions = {},
): PagedAsyncIterableIterator<SecretAttributes> {
const iter = this.listSecretsAll(options);
const iter = listSecretsAll(options);
return {
async next() {
return iter.next();
},
[Symbol.asyncIterator]() {
return this;
},
byPage: (settings: PageSettings = {}) => this.listSecretsPage(settings, options),
byPage: (settings: PageSettings = {}) => listSecretsPage(settings, options),
};
}
for await (const page of listSecrets().byPage({ maxPageSize: 2 })) {
Expand Down
19 changes: 16 additions & 3 deletions sdk/core/core-paging/test/snippets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,34 @@
// Licensed under the MIT License.

import { describe, it } from "vitest";
import { PagedAsyncIterableIterator, PageSettings } from "@azure/core-paging";

interface ListSecretsOptions {}
interface SecretAttributes {}
function listSecretsAll(options: ListSecretsOptions): AsyncIterableIterator<SecretAttributes> {
throw "stub";
}
function listSecretsPage(
pageSettings: PageSettings,
options: ListSecretsOptions,
): AsyncIterableIterator<SecretAttributes[]> {
throw "stub";
}

describe("snippets", () => {
it("paging_example", () => {
it("paging_example", async () => {
function listSecrets(
options: ListSecretsOptions = {},
): PagedAsyncIterableIterator<SecretAttributes> {
const iter = this.listSecretsAll(options);
const iter = listSecretsAll(options);
return {
async next() {
return iter.next();
},
[Symbol.asyncIterator]() {
return this;
},
byPage: (settings: PageSettings = {}) => this.listSecretsPage(settings, options),
byPage: (settings: PageSettings = {}) => listSecretsPage(settings, options),
};
}

Expand Down
33 changes: 26 additions & 7 deletions sdk/core/core-rest-pipeline/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,19 @@ A `PipelineResponse` describes the HTTP response (body, headers, and status code
A `SendRequest` method is a method that given a `PipelineRequest` can asynchronously return a `PipelineResponse`.

```ts snippet:send_request
export type SendRequest = (request: PipelineRequest) => Promise<PipelineResponse>;
import { PipelineRequest, PipelineResponse } from "@azure/core-rest-pipeline";

type SendRequest = (request: PipelineRequest) => Promise<PipelineResponse>;
```

### HttpClient

An `HttpClient` is any object that satisfies the following interface to implement a `SendRequest` method:

```ts snippet:http_request
export interface HttpClient {
import { SendRequest } from "@azure/core-rest-pipeline";

interface HttpClient {
/**
* The method that makes the request and returns a response.
*/
Expand All @@ -55,7 +59,9 @@ export interface HttpClient {
A `PipelinePolicy` is a simple object that implements the following interface:

```ts snippet:pipeline_policy
export interface PipelinePolicy {
import { PipelineRequest, SendRequest, PipelineResponse } from "@azure/core-rest-pipeline";

interface PipelinePolicy {
/**
* The policy name. Must be a unique string in the pipeline.
*/
Expand All @@ -76,13 +82,15 @@ One can view the role of policies as that of `middleware`, a concept that is fam
The `sendRequest` implementation can both transform the outgoing request as well as the incoming response:

```ts snippet:custom_policy
import { PipelineRequest, SendRequest, PipelineResponse } from "@azure/core-rest-pipeline";

const customPolicy = {
name: "My wonderful policy",
async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
// Change the outgoing request by adding a new header
request.headers.set("X-Cool-Header", 42);
const result = await next(request);
if (response.status === 403) {
if (result.status === 403) {
// Do something special if this policy sees Forbidden
}
return result;
Expand All @@ -101,8 +109,17 @@ You can think of policies being applied like a stack (first-in/last-out.) The fi
A `Pipeline` satisfies the following interface:

```ts snippet:pipeline
export interface Pipeline {
addPolicy(policy: PipelinePolicy, options?: AddPolicyOptions): void;
import {
PipelinePolicy,
AddPipelineOptions,
PipelinePhase,
HttpClient,
PipelineRequest,
PipelineResponse,
} from "@azure/core-rest-pipeline";

interface Pipeline {
addPolicy(policy: PipelinePolicy, options?: AddPipelineOptions): void;
removePolicy(options: { name?: string; phase?: PipelinePhase }): PipelinePolicy[];
sendRequest(httpClient: HttpClient, request: PipelineRequest): Promise<PipelineResponse>;
getOrderedPolicies(): PipelinePolicy[];
Expand All @@ -124,7 +141,9 @@ Phases occur in the above order, with serialization policies being applied first
When adding a policy to the pipeline you can specify not only what phase a policy is in, but also if it has any dependencies:

```ts snippet:add_policy_options
export interface AddPolicyOptions {
import { PipelinePhase } from "@azure/core-rest-pipeline";

interface AddPipelineOptions {
beforePolicies?: string[];
afterPolicies?: string[];
afterPhase?: PipelinePhase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe("userAgentPlatform", () => {
});

it("should handle an empty process.versions", async () => {
vi.mocked(process).versions = undefined;
(vi.mocked(process) as any).versions = undefined;
const map = new Map<string, string>();

await setPlatformSpecificData(map);
Expand All @@ -31,7 +31,7 @@ describe("userAgentPlatform", () => {
});

it("should handle a Node.js process.versions with Bun", async () => {
vi.mocked(process).versions = { bun: "1.0.0" };
(vi.mocked(process) as any).versions = { bun: "1.0.0" };
const map = new Map<string, string>();

await setPlatformSpecificData(map);
Expand All @@ -44,7 +44,7 @@ describe("userAgentPlatform", () => {
});

it("should handle a Node.js process.versions with Deno", async () => {
vi.mocked(process).versions = { deno: "2.0.0" };
(vi.mocked(process) as any).versions = { deno: "2.0.0" };
const map = new Map<string, string>();

await setPlatformSpecificData(map);
Expand All @@ -57,7 +57,7 @@ describe("userAgentPlatform", () => {
});

it("should handle a Node.js process.versions", async () => {
vi.mocked(process).versions = { node: "20.0.0" };
(vi.mocked(process) as any).versions = { node: "20.0.0" };
const map = new Map<string, string>();

await setPlatformSpecificData(map);
Expand Down
Loading

0 comments on commit cf8d25f

Please sign in to comment.