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

[Issue 17826] Fix ES Lint errors in common\tools\dev\tool #18819

Merged
merged 18 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/commands/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const baseCommandInfo = makeCommandInfo("dev-tool", "Azure SDK for JS dev
/**
* Default dev-tool subcommand
*/
export const baseCommand = async (...args: string[]) => {
export const baseCommand = async (...args: string[]): Promise<void> => {
const status = await subCommand(baseCommandInfo, baseCommands)(...args);

if (!status) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,30 +87,30 @@ function buildRunSamplesScript(
const scriptContent = `#!/bin/sh

function install_dependencies_helper() {
local samples_path=\$1;
local samples_path=$1;
cd \${samples_path};
${compileCMD(`npm install ${artifactURL}`, printToScreen)}
${compileCMD(`npm install`, printToScreen)}
}

function install_packages() {
echo "Using node \$(node -v) to install dependencies";
echo "Using node $(node -v) to install dependencies";
install_dependencies_helper ${samplesDir}/javascript
install_dependencies_helper ${samplesDir}/typescript;
cp ${envFilePath} ${samplesDir}/javascript/;
}

function run_samples() {
samples_path=\$1;
echo "Using node \$(node -v) to run samples in \${samples_path}";
samples_path=$1;
echo "Using node $(node -v) to run samples in \${samples_path}";
cd "\${samples_path}";
for SAMPLE in *.js; do
node \${SAMPLE};
done
}

function build_typescript() {
echo "Using node \$(node -v) to build the typescript samples";
echo "Using node $(node -v) to build the typescript samples";
cd ${samplesDir}/typescript
${compileCMD(`npm run build`, printToScreen)}
cp ${envFilePath} ${samplesDir}/typescript/dist/
Expand Down Expand Up @@ -273,7 +273,7 @@ export default leafCommand(commandInfo, async (options) => {
options["node-versions"]
?.split(",")
.concat(options["node-version"])
.filter((ver) => ver !== "" && parseInt(ver) !== NaN)
.filter((ver) => ver !== "" && !isNaN(parseInt(ver)))
)
];
const dockerContextDirectory: string =
Expand Down
4 changes: 2 additions & 2 deletions common/tools/dev-tool/src/commands/samples/prep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ async function enableLocalRun(
// that can escape both linux and windows path separators
const depth = relativeDir.length - relativeDir.split(path.sep).join("").length;

let relativePath = new Array(depth).fill("..").join("/");
const relativePath = new Array(depth).fill("..").join("/");

outputContent = fileContents.replace(
importRegex,
Expand Down Expand Up @@ -112,7 +112,7 @@ export default leafCommand(commandInfo, async (options) => {
const jsDir = path.join(outputDir, "javascript");
const jsFiles = findMatchingFiles(jsDir, (name, entry) => entry.isFile() && name.endsWith(".js"));

for await (const fileName of cat(tsFiles, jsFiles)) {
for await (const fileName of cat(tsFiles, jsFiles) as unknown as string) {
await enableLocalRun(fileName, pkg.path, pkg.name, options["use-packages"]);
}

Expand Down
38 changes: 19 additions & 19 deletions common/tools/dev-tool/src/commands/samples/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ function createPackageJson(info: SampleGenerationInfo, outputKind: OutputKind):
},
...(outputKind === OutputKind.TypeScript
? {
// We only include these in TypeScript
scripts: {
build: "tsc",
prebuild: "rimraf dist/"
}
// We only include these in TypeScript
scripts: {
build: "tsc",
prebuild: "rimraf dist/"
}
}
: {}),
repository: {
type: "git",
Expand Down Expand Up @@ -122,7 +122,7 @@ function isDependency(moduleSpecifier: string): boolean {

// This seems like a reasonable test for "is a relative path" as long as
// absolute path imports are forbidden.
const isRelativePath = /^\.\.?[\/\\]/.test(moduleSpecifier);
const isRelativePath = /^\.\.?[/\\]/.test(moduleSpecifier);
return !isRelativePath;
}

Expand Down Expand Up @@ -326,7 +326,7 @@ async function makeSampleGenerationInfo(
return undefined as never;
}

const moduleInfos = await processSources(projectInfo, sampleSources, fail);
const moduleInfos = await processSources(projectInfo, sampleSources as string[], fail);
Copy link
Member

Choose a reason for hiding this comment

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

why is this type cast needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I have fixed it. It is redundant after I added AsyncGenerator<string> to common/tools/dev-tool/src/util/findMatchingFiles.ts


const defaultDependencies: Record<string, string> = {
// If we are a beta package, use "next", otherwise we will use "latest"
Expand Down Expand Up @@ -430,14 +430,14 @@ async function makeSampleGenerationInfo(
}, defaultDependencies),
...(outputKind === OutputKind.TypeScript
? {
// In TypeScript samples, we include TypeScript and `rimraf`, because they're used
// in the package scripts.
devDependencies: {
...typesDependencies,
typescript: devToolPackageJson.dependencies.typescript,
rimraf: "latest"
}
// In TypeScript samples, we include TypeScript and `rimraf`, because they're used
// in the package scripts.
devDependencies: {
...typesDependencies,
typescript: devToolPackageJson.dependencies.typescript,
rimraf: "latest"
}
}
: {})
};
}
Expand All @@ -455,11 +455,11 @@ function createReadme(outputKind: OutputKind, info: SampleGenerationInfo): strin
frontmatter: info.disableDocsMs
? undefined
: {
page_type: "sample",
languages: [fullOutputKind],
products: info.productSlugs,
urlFragment: `${info.baseName}-${fullOutputKind}`
},
page_type: "sample",
languages: [fullOutputKind],
products: info.productSlugs,
urlFragment: `${info.baseName}-${fullOutputKind}`
},
publicationDirectory: PUBLIC_SAMPLES_BASE + "/" + info.topLevelDirectory,
useTypeScript: outputKind === OutputKind.TypeScript,
...info,
Expand Down
6 changes: 3 additions & 3 deletions common/tools/dev-tool/src/commands/samples/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export const commandInfo = makeCommandInfo(
async function runSingle(name: string, accumulatedErrors: Array<[string, string]>) {
log("Running", name);
try {
if (/.*[\\\/]samples(-dev)?[\\\/].*/.exec(name)) {
if (/.*[\\/]samples(-dev)?[\\/].*/.exec(name)) {
// This is an un-prepared sample, so just require it and it will run.
await import(name);
} else if (!/.*[\\\/]dist-samples[\\\/].*/.exec(name)) {
} else if (!/.*[\\/]dist-samples[\\/].*/.exec(name)) {
// This is not an unprepared or a prepared sample
log.error("Skipped. This file is not in any samples folder.");
} else {
Expand Down Expand Up @@ -90,7 +90,7 @@ export default leafCommand(commandInfo, async (options) => {
skips
}
)) {
await runSingle(fileName, errors);
await runSingle(fileName as string, errors);
}
} else {
log.warn(`Sample ${sample} is neither a file nor a directory.`);
Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/commands/samples/tsToJs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const commandInfo = makeCommandInfo(
);

const prettierOptions: prettier.Options = {
...(require("../../../../eslint-plugin-azure-sdk/prettier.json") as prettier.Options),
...(import("../../../../eslint-plugin-azure-sdk/prettier.json") as prettier.Options),
parser: "typescript"
};

Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/commands/test-proxy/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const commandInfo = makeCommandInfo(
{}
);

export default leafCommand(commandInfo, async (_options) => {
export default leafCommand(commandInfo, async () => {
await startProxyTool();
return true;
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const commandInfo = makeCommandInfo(
{}
);

export default leafCommand(commandInfo, async (_options) => {
export default leafCommand(commandInfo, async () => {
const result = await checkWithTimeout(isProxyToolActive, 1000, 120000);
return result;
});
24 changes: 12 additions & 12 deletions common/tools/dev-tool/src/config/rollup.base.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ export function openTelemetryCommonJs(): Record<string, string[]> {
// working around a limitation in the rollup common.js plugin - it's not able to resolve these modules so the named exports listed above will not get applied. We have to drill down to the actual path.
`../../../common/temp/node_modules/.pnpm/@opentelemetry+api@${version}/node_modules/@opentelemetry/api/build/src/index.js`
] = [
"SpanKind",
"TraceFlags",
"getSpan",
"setSpan",
"StatusCode",
"CanonicalCode",
"getSpanContext",
"setSpanContext"
];
"SpanKind",
"TraceFlags",
"getSpan",
"setSpan",
"StatusCode",
"CanonicalCode",
"getSpanContext",
"setSpanContext"
];
}

return namedExports;
Expand All @@ -72,7 +72,7 @@ function ignoreNiseSinonEvalWarnings(warning: RollupWarning): boolean {
return (
warning.code === "EVAL" &&
(warning.id?.includes("node_modules/nise") || warning.id?.includes("node_modules/sinon")) ===
true
true
);
}

Expand Down Expand Up @@ -103,7 +103,7 @@ function makeOnWarnForTesting(): (warning: RollupWarning, warn: WarningHandler)

// #endregion

export function makeBrowserTestConfig() {
export function makeBrowserTestConfig(): RollupOptions {
const config: RollupOptions = {
input: {
include: ["dist-esm/test/**/*.spec.js"],
Expand Down Expand Up @@ -151,7 +151,7 @@ const defaultConfigurationOptions: ConfigurationOptions = {
disableBrowserBundle: false
};

export function makeConfig(pkg: PackageJson, options?: Partial<ConfigurationOptions>) {
export function makeConfig(pkg: PackageJson, options?: Partial<ConfigurationOptions>): RollupOptions[] {
options = {
...defaultConfigurationOptions,
...(options ?? {})
Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/framework/CommandModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ export interface CommandModule<Options extends CommandOptions> {
/**
* A map from command name to an async function that loads its module
*/
export type CommandLoader = { [k: string]: () => Promise<CommandModule<{}>> };
export type CommandLoader = { [k: string]: () => Promise<CommandModule<CommandOptions>> };
71 changes: 34 additions & 37 deletions common/tools/dev-tool/src/templates/sampleReadme.md.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ function resourceLinks(info: SampleReadmeConfiguration) {
function resources(info: SampleReadmeConfiguration) {
const resources = Object.entries(info.requiredResources ?? {});

const header = `You need [an Azure subscription][freesub] ${
resources.length > 0 ? "and the following Azure resources " : ""
}to run these sample programs${resources.length > 0 ? ":\n\n" : "."}`;
const header = `You need [an Azure subscription][freesub] ${resources.length > 0 ? "and the following Azure resources " : ""
}to run these sample programs${resources.length > 0 ? ":\n\n" : "."}`;

return (
header + resources.map(([name]) => `- [${name}][${resourceNameToLinkSlug(name)}]`).join("\n")
Expand Down Expand Up @@ -137,15 +136,14 @@ function exampleNodeInvocation(info: SampleReadmeConfiguration) {
.map((envVar) => `${envVar}="<${envVar.replace(/_/g, " ").toLowerCase()}>"`)
.join(" ");

return `${envVars} node ${
info.useTypeScript ? "dist/" : ""
}${firstModule.relativeSourcePath.replace(/\.ts$/, ".js")}`;
return `${envVars} node ${info.useTypeScript ? "dist/" : ""
}${firstModule.relativeSourcePath.replace(/\.ts$/, ".js")}`;
}

/**
* Creates a README for a sample package from a SampleReadmeConfiguration.
*/
export default (info: SampleReadmeConfiguration) => {
export default (info: SampleReadmeConfiguration): string => {
let stepCount = 1;
const step = (content: string) => `${stepCount++}. ${content}`;

Expand All @@ -157,8 +155,7 @@ export default (info: SampleReadmeConfiguration) => {

${info.customSnippets?.header ?? ""}

These sample programs show how to use the ${language} client libraries for ${
info.productName
These sample programs show how to use the ${language} client libraries for ${info.productName
} in some common scenarios.

${table(info)}
Expand All @@ -168,17 +165,17 @@ ${table(info)}
The sample programs are compatible with [LTS versions of Node.js](https://nodejs.org/about/releases/).

${(() => {
if (info.useTypeScript) {
return [
"Before running the samples in Node, they must be compiled to JavaScript using the TypeScript compiler. For more information on TypeScript, see the [TypeScript documentation][typescript]. Install the TypeScript compiler using:",
"",
fence("bash", "npm install -g typescript"),
""
].join("\n");
} else {
return "";
}
})()}\
if (info.useTypeScript) {
return [
"Before running the samples in Node, they must be compiled to JavaScript using the TypeScript compiler. For more information on TypeScript, see the [TypeScript documentation][typescript]. Install the TypeScript compiler using:",
"",
fence("bash", "npm install -g typescript"),
""
].join("\n");
} else {
return "";
}
})()}\
${resources(info)}

${info.customSnippets?.prerequisites ?? ""}
Expand All @@ -195,28 +192,28 @@ ${step("Install the dependencies using `npm`:")}

${fence("bash", "npm install")}
${(() => {
if (info.useTypeScript) {
return [step("Compile the samples:"), "", fence("bash", "npm run build"), ""].join("\n");
} else {
return "";
}
})()}
if (info.useTypeScript) {
return [step("Compile the samples:"), "", fence("bash", "npm run build"), ""].join("\n");
} else {
return "";
}
})()}
${step(
"Edit the file `sample.env`, adding the correct credentials to access the Azure service and run the samples. Then rename the file from `sample.env` to just `.env`. The sample programs will read this file automatically."
)}
"Edit the file `sample.env`, adding the correct credentials to access the Azure service and run the samples. Then rename the file from `sample.env` to just `.env`. The sample programs will read this file automatically."
)}

${step(
"Run whichever samples you like (note that some samples may require additional setup, see the table above):"
)}
"Run whichever samples you like (note that some samples may require additional setup, see the table above):"
)}

${fence(
"bash",
`node ${(() => {
const firstSource = filterModules(info)[0].relativeSourcePath;
const filePath = info.useTypeScript ? "dist/" : "";
return filePath + firstSource.replace(/\.ts$/, ".js");
})()}`
)}
"bash",
`node ${(() => {
const firstSource = filterModules(info)[0].relativeSourcePath;
const filePath = info.useTypeScript ? "dist/" : "";
return filePath + firstSource.replace(/\.ts$/, ".js");
})()}`
)}

Alternatively, run a single sample with the correct environment variables set (setting up the \`.env\` file is not required if you do this), for example (cross-platform):

Expand Down
4 changes: 2 additions & 2 deletions common/tools/dev-tool/src/util/checkWithTimeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ const log = createPrinter("check-with-timeout");
*/
export async function checkWithTimeout(
predicate: () => boolean | Promise<boolean>,
delayBetweenRetriesInMilliseconds: number = 1000,
maxWaitTimeInMilliseconds: number = 10000
delayBetweenRetriesInMilliseconds = 1000,
maxWaitTimeInMilliseconds = 10000
): Promise<boolean> {
const maxTime = Date.now() + maxWaitTimeInMilliseconds;
while (Date.now() < maxTime) {
Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/util/findMatchingFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function* findMatchingFiles(
dir: string,
matches: (name: string, entry: fs.Stats) => boolean,
findOptions?: Partial<FindOptions>
) {
): AsyncGenerator {
const q: FileInfo[] = [];

const options: FindOptions = { ...defaultFindOptions, ...findOptions };
Expand Down
2 changes: 1 addition & 1 deletion common/tools/dev-tool/src/util/sampleConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ const removeJsTsExtensions = (name: string): string => name.replace(/\.[jt]s$/,
* @param info - FileInfo of a sample file to be considered
* @param skips - a list of strings that identify files to be skipped
*/
export function shouldSkip(info: FileInfo, skips: string[]) {
export function shouldSkip(info: FileInfo, skips: string[]): boolean {
// Add a slash to the skip if necessary
const addFirstSlash = (skip: string): string => (skip.startsWith("/") ? skip : "/" + skip);

Expand Down
Loading