Skip to content

Commit

Permalink
improvement(generate:typetests): Code cleanup and other fixes (#20905)
Browse files Browse the repository at this point in the history
Contains the following fixes and improvements to generate:typetests:

- The `level` flag is now a custom flag type that returns an ApiLevel
type.
- The type test header comment now correctly refers to the build-cli
package.
- The exports handling fallback logic now uses the correct priority:
imports condition, requires condition, and only then does it fall back
to the types/typings fields.
- The module resolution override in the ts-morph projects was
unnecessary and has been removed.
- Instead of adding all declaration files to the ts-morph project then
fishing out the individual file, now the single file is added to the
project and used.
  • Loading branch information
tylerbutler authored May 3, 2024
1 parent 3d230b4 commit 350c06a
Showing 1 changed file with 21 additions and 22 deletions.
43 changes: 21 additions & 22 deletions build-tools/packages/build-cli/src/commands/generate/typetests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { PackageName } from "@rushstack/node-core-library";
import * as changeCase from "change-case";
import { mkdirSync, readJson, rmSync, writeFileSync } from "fs-extra";
import * as resolve from "resolve.exports";
import { ModuleKind, ModuleResolutionKind, Project, type SourceFile } from "ts-morph";
import { ModuleKind, Project, type SourceFile } from "ts-morph";
import { PackageCommand } from "../../BasePackageCommand";
import { ApiLevel, ensureDevDependencyExists, knownApiLevels } from "../../library";
import { unscopedPackageNameString } from "./entrypoints";
Expand All @@ -33,11 +33,11 @@ export default class GenerateTypetestsCommand extends PackageCommand<
static readonly description = "Generates type tests for a package or group of packages.";

static readonly flags = {
level: Flags.string({
level: Flags.custom<ApiLevel>({
description: "What API level to generate tests for.",
default: ApiLevel.internal,
options: knownApiLevels,
}),
})(),
outDir: Flags.directory({
description: "Where to emit the type tests file.",
default: "./src/test/types",
Expand All @@ -55,10 +55,7 @@ export default class GenerateTypetestsCommand extends PackageCommand<
} as const;

protected async processPackage(pkg: Package): Promise<void> {
const { outDir, outFile } = this.flags;

// This cast is safe because oclif has already ensured only known ApiLevel values get to this point.
const level = this.flags.level as ApiLevel;
const { level, outDir, outFile } = this.flags;
const fallbackLevel = this.flags.publicFallback ? ApiLevel.public : undefined;

// Do not check that file exists before opening:
Expand Down Expand Up @@ -148,7 +145,7 @@ export default class GenerateTypetestsCommand extends PackageCommand<
/*
* THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
* Generated by fluid-type-test-generator in @fluidframework/build-tools.
* Generated by flub generate:typetests in @fluid-tools/build-cli.
*/
import type * as old from "${previousPackageName}${
Expand Down Expand Up @@ -226,14 +223,20 @@ export function getTypesPathFromPackage(
// First try to resolve with the "import" condition, assuming the package is either ESM-only or dual-format.
// conditions: ["default", "types", "import", "node"]
const exports = resolve.exports(packageJson, entrypoint, { conditions: ["types"] });
typesPath =
exports === undefined || exports.length === 0
? packageJson.types ?? packageJson.typings
: exports[0];

// resolve.exports returns a `Exports.Output | void` type, though the documentation isn't clear under what
// conditions `void` would be the return type vs. just throwing an exception. Since the types say exports could be
// undefined or an empty array (Exports.Output is an array type), check for those conditions.
typesPath = exports === undefined || exports.length === 0 ? undefined : exports[0];
} catch {
// Catch and ignore any exceptions here; we'll retry with the require condition.
}

// Found the types using the import condition, so return early.
if (typesPath !== undefined) {
return typesPath;
}

try {
// If nothing is found when using the "import" condition, try the "require" condition. It may be possible to do this
// in a single call to resolve.exports, but the documentation is a little unclear. This seems a safe, if inelegant
Expand All @@ -243,16 +246,13 @@ export function getTypesPathFromPackage(
conditions: ["types"],
require: true,
});
// Only assign typesPath if it wasn't already assigned earlier.
typesPath ??=
exports === undefined || exports.length === 0
? packageJson.types ?? packageJson.typings
: exports[0];
typesPath = exports === undefined || exports.length === 0 ? undefined : exports[0];
} catch {
// Catch any exceptions here; we'll return undefined instead of throwing them.
}

return typesPath;
// Fall back to types/typings fields if both import and require conditions yielded nothing.
return typesPath ?? packageJson.types ?? packageJson.typings;
}

/**
Expand Down Expand Up @@ -309,19 +309,18 @@ function typeDataFromFile(file: SourceFile, log: Logger): Map<string, TypeData>
/**
* Loads a ts-morph source file from the provided path.
*
* @param typesPath - The path to the types file to load. This path is expected to be relative to
* @param typesPath - The path to the types file to load.
* @returns The loaded source file.
*/
export function loadTypesSourceFile(typesPath: string): SourceFile {
// Note that this does NOT load anything from tsconfig.
const project = new Project({
skipAddingFilesFromTsConfig: true,
compilerOptions: {
module: ModuleKind.Node16,
moduleResolution: ModuleResolutionKind.Node16,
},
});
project.addSourceFilesAtPaths(`${path.dirname(typesPath)}/**/*.d.*ts`);
const sourceFile = project.getSourceFileOrThrow(path.basename(typesPath));
const sourceFile = project.addSourceFileAtPath(typesPath);
return sourceFile;
}

Expand Down

0 comments on commit 350c06a

Please sign in to comment.