Address PR review comments
- Refactor targetPaths/findOwnPaths to class-based implementations with unified caching and .dir/.rootDir properties alongside resolve methods - Replace jest.mock with jest.spyOn in plugin-manager.test.ts - Remove paths compatibility wrapper from repo-tools, migrate all internal consumers to use targetPaths from @backstage/cli-common directly - Fix changeset package name (@techdocs/cli not @backstage/techdocs-cli) - Add migration examples to cli-common changeset - Update API report for cli-common Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
+1
-1
@@ -32,7 +32,7 @@ export class SuccessCache {
|
||||
* location.
|
||||
*/
|
||||
static trimPaths(input: string) {
|
||||
return input.replaceAll(targetPaths.resolveRoot(), '');
|
||||
return input.replaceAll(targetPaths.rootDir, '');
|
||||
}
|
||||
|
||||
constructor(name: string, basePath?: string) {
|
||||
|
||||
@@ -47,14 +47,14 @@ export async function command(opts: OptionValues): Promise<void> {
|
||||
|
||||
if (role === 'frontend') {
|
||||
return buildFrontend({
|
||||
targetDir: targetPaths.resolve(),
|
||||
targetDir: targetPaths.dir,
|
||||
configPaths,
|
||||
writeStats: Boolean(opts.stats),
|
||||
webpack,
|
||||
});
|
||||
}
|
||||
return buildBackend({
|
||||
targetDir: targetPaths.resolve(),
|
||||
targetDir: targetPaths.dir,
|
||||
configPaths,
|
||||
skipBuildDependencies: Boolean(opts.skipBuildDependencies),
|
||||
minify: Boolean(opts.minify),
|
||||
@@ -77,7 +77,7 @@ export async function command(opts: OptionValues): Promise<void> {
|
||||
if (isModuleFederationRemote) {
|
||||
console.log('Building package as a module federation remote');
|
||||
return buildFrontend({
|
||||
targetDir: targetPaths.resolve(),
|
||||
targetDir: targetPaths.dir,
|
||||
configPaths: [],
|
||||
writeStats: Boolean(opts.stats),
|
||||
isModuleFederationRemote,
|
||||
|
||||
@@ -25,7 +25,7 @@ export async function command(opts: OptionValues): Promise<void> {
|
||||
await startPackage({
|
||||
role: await findRoleFromCommand(opts),
|
||||
entrypoint: opts.entrypoint,
|
||||
targetDir: targetPaths.resolve(),
|
||||
targetDir: targetPaths.dir,
|
||||
configPaths: opts.config as string[],
|
||||
checksEnabled: Boolean(opts.check),
|
||||
linkedWorkspace: await resolveLinkedWorkspace(opts.link),
|
||||
|
||||
@@ -44,7 +44,7 @@ export async function startBackend(options: StartBackendOptions) {
|
||||
|
||||
export async function startBackendPlugin(options: StartBackendOptions) {
|
||||
const hasDevIndexEntry = await fs.pathExists(
|
||||
resolvePath(options.targetDir ?? targetPaths.resolve(), 'dev/index.ts'),
|
||||
resolvePath(options.targetDir ?? targetPaths.dir, 'dev/index.ts'),
|
||||
);
|
||||
if (!hasDevIndexEntry) {
|
||||
console.warn(
|
||||
|
||||
@@ -39,7 +39,7 @@ interface StartAppOptions {
|
||||
|
||||
export async function startFrontend(options: StartAppOptions) {
|
||||
const packageJson = (await readJson(
|
||||
resolvePath(options.targetDir ?? targetPaths.resolve(), 'package.json'),
|
||||
resolvePath(options.targetDir ?? targetPaths.dir, 'package.json'),
|
||||
)) as BackstagePackageJson;
|
||||
|
||||
if (!hasReactDomClient()) {
|
||||
@@ -59,7 +59,7 @@ export async function startFrontend(options: StartAppOptions) {
|
||||
moduleFederationRemote: options.isModuleFederationRemote
|
||||
? await getModuleFederationRemoteOptions(
|
||||
packageJson,
|
||||
resolvePath(targetPaths.resolve()),
|
||||
resolvePath(targetPaths.dir),
|
||||
)
|
||||
: undefined,
|
||||
});
|
||||
|
||||
@@ -90,7 +90,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
|
||||
targetDir: pkg.dir,
|
||||
packageJson: pkg.packageJson,
|
||||
outputs,
|
||||
logPrefix: `${chalk.cyan(relativePath(targetPaths.resolveRoot(), pkg.dir))}: `,
|
||||
logPrefix: `${chalk.cyan(relativePath(targetPaths.rootDir, pkg.dir))}: `,
|
||||
workspacePackages: packages,
|
||||
minify: opts.minify ?? buildOptions.minify,
|
||||
};
|
||||
|
||||
@@ -117,7 +117,7 @@ export async function makeRollupConfigs(
|
||||
options: BuildOptions,
|
||||
): Promise<RollupOptions[]> {
|
||||
const configs = new Array<RollupOptions>();
|
||||
const targetDir = options.targetDir ?? targetPaths.resolve();
|
||||
const targetDir = options.targetDir ?? targetPaths.dir;
|
||||
|
||||
let targetPkg = options.packageJson;
|
||||
if (!targetPkg) {
|
||||
@@ -287,7 +287,7 @@ export async function makeRollupConfigs(
|
||||
e.name,
|
||||
targetPaths.resolveRoot(
|
||||
'dist-types',
|
||||
relativePath(targetPaths.resolveRoot(), targetDir),
|
||||
relativePath(targetPaths.rootDir, targetDir),
|
||||
e.path.replace(/\.(?:ts|tsx)$/, '.d.ts'),
|
||||
),
|
||||
]),
|
||||
|
||||
@@ -34,7 +34,7 @@ export function formatErrorMessage(error: any) {
|
||||
msg += `\n\n`;
|
||||
for (const { text, location } of error.errors) {
|
||||
const { line, column } = location;
|
||||
const path = relativePath(targetPaths.resolve(), error.id);
|
||||
const path = relativePath(targetPaths.dir, error.id);
|
||||
const loc = chalk.cyan(`${path}:${line}:${column}`);
|
||||
|
||||
if (text === 'Unexpected "<"' && error.id.endsWith('.js')) {
|
||||
@@ -107,7 +107,7 @@ export const buildPackage = async (options: BuildOptions) => {
|
||||
|
||||
const rollupConfigs = await makeRollupConfigs(options);
|
||||
|
||||
const targetDir = options.targetDir ?? targetPaths.resolve();
|
||||
const targetDir = options.targetDir ?? targetPaths.dir;
|
||||
await fs.remove(resolvePath(targetDir, 'dist'));
|
||||
|
||||
const buildTasks = rollupConfigs.map(rollupBuild);
|
||||
|
||||
@@ -20,7 +20,7 @@ import { targetPaths } from '@backstage/cli-common';
|
||||
export function hasReactDomClient() {
|
||||
try {
|
||||
require.resolve('react-dom/client', {
|
||||
paths: [targetPaths.resolve()],
|
||||
paths: [targetPaths.dir],
|
||||
});
|
||||
return true;
|
||||
} catch {
|
||||
|
||||
@@ -53,7 +53,7 @@ export async function createWorkspaceLinkingPlugins(
|
||||
/^react(?:-router)?(?:-dom)?$/,
|
||||
resource => {
|
||||
if (!relativePath(linkedRoot.dir, resource.context).startsWith('..')) {
|
||||
resource.context = targetPaths.resolve();
|
||||
resource.context = targetPaths.dir;
|
||||
}
|
||||
},
|
||||
),
|
||||
|
||||
@@ -147,7 +147,7 @@ export async function createDetectedModulesEntryPoint(options: {
|
||||
// Previous versions of the CLI would write the detected modules file to the
|
||||
// root `node_modules`, this makes sure that doesn't exist to minimize risk of conflicts
|
||||
const legacyDetectedModulesPath = joinPath(
|
||||
targetPaths.resolveRoot(),
|
||||
targetPaths.rootDir,
|
||||
'node_modules',
|
||||
`${DETECTED_MODULES_MODULE_NAME}.js`,
|
||||
);
|
||||
|
||||
@@ -21,14 +21,14 @@ import { targetPaths, findOwnPaths } from '@backstage/cli-common';
|
||||
export type BundlingPathsOptions = {
|
||||
// bundle entrypoint, e.g. 'src/index'
|
||||
entry: string;
|
||||
// Target directory, defaulting to targetPaths.resolve()
|
||||
// Target directory, defaulting to targetPaths.dir
|
||||
targetDir?: string;
|
||||
// Relative dist directory, defaulting to 'dist'
|
||||
dist?: string;
|
||||
};
|
||||
|
||||
export function resolveBundlingPaths(options: BundlingPathsOptions) {
|
||||
const { entry, targetDir = targetPaths.resolve() } = options;
|
||||
const { entry, targetDir = targetPaths.dir } = options;
|
||||
|
||||
const resolveTargetModule = (pathString: string) => {
|
||||
for (const ext of ['mjs', 'js', 'ts', 'tsx', 'jsx']) {
|
||||
@@ -70,7 +70,7 @@ export function resolveBundlingPaths(options: BundlingPathsOptions) {
|
||||
targetTsConfig: targetPaths.resolveRoot('tsconfig.json'),
|
||||
targetPackageJson: resolvePath(targetDir, 'package.json'),
|
||||
rootNodeModules: targetPaths.resolveRoot('node_modules'),
|
||||
root: targetPaths.resolveRoot(),
|
||||
root: targetPaths.rootDir,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -54,7 +54,7 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be
|
||||
checkReactVersion();
|
||||
|
||||
const { name } = await fs.readJson(
|
||||
resolvePath(options.targetDir ?? targetPaths.resolve(), 'package.json'),
|
||||
resolvePath(options.targetDir ?? targetPaths.dir, 'package.json'),
|
||||
);
|
||||
|
||||
let devServer: RspackDevServer | undefined = undefined;
|
||||
@@ -272,7 +272,7 @@ function checkReactVersion() {
|
||||
try {
|
||||
// Make sure we're looking at the root of the target repo
|
||||
const reactPkgPath = require.resolve('react/package.json', {
|
||||
paths: [targetPaths.resolveRoot()],
|
||||
paths: [targetPaths.rootDir],
|
||||
});
|
||||
const reactPkg = require(reactPkgPath);
|
||||
if (reactPkg.version.startsWith('16.')) {
|
||||
|
||||
@@ -211,7 +211,7 @@ export async function createDistWorkspace(
|
||||
targetDir: pkg.dir,
|
||||
packageJson: pkg.packageJson,
|
||||
outputs: outputs,
|
||||
logPrefix: `${chalk.cyan(relativePath(targetPaths.resolveRoot(), pkg.dir))}: `,
|
||||
logPrefix: `${chalk.cyan(relativePath(targetPaths.rootDir, pkg.dir))}: `,
|
||||
minify: options.minify,
|
||||
workspacePackages: packages,
|
||||
});
|
||||
@@ -252,7 +252,7 @@ export async function createDistWorkspace(
|
||||
if (options.skeleton) {
|
||||
const skeletonFiles = targets
|
||||
.map(target => {
|
||||
const dir = relativePath(targetPaths.resolveRoot(), target.dir);
|
||||
const dir = relativePath(targetPaths.rootDir, target.dir);
|
||||
return joinPath(dir, 'package.json');
|
||||
})
|
||||
.sort();
|
||||
@@ -301,7 +301,7 @@ async function moveToDistWorkspace(
|
||||
fastPackPackages.map(async target => {
|
||||
console.log(`Moving ${target.name} into dist workspace`);
|
||||
|
||||
const outputDir = relativePath(targetPaths.resolveRoot(), target.dir);
|
||||
const outputDir = relativePath(targetPaths.rootDir, target.dir);
|
||||
const absoluteOutputPath = resolvePath(workspaceDir, outputDir);
|
||||
await productionPack({
|
||||
packageDir: target.dir,
|
||||
@@ -321,7 +321,7 @@ async function moveToDistWorkspace(
|
||||
cwd: target.dir,
|
||||
}).waitForExit();
|
||||
|
||||
const outputDir = relativePath(targetPaths.resolveRoot(), target.dir);
|
||||
const outputDir = relativePath(targetPaths.rootDir, target.dir);
|
||||
const absoluteOutputPath = resolvePath(workspaceDir, outputDir);
|
||||
await fs.ensureDir(absoluteOutputPath);
|
||||
|
||||
|
||||
@@ -23,6 +23,12 @@ const mockDir = createMockDirectory();
|
||||
jest.mock('@backstage/cli-common', () => ({
|
||||
...jest.requireActual('@backstage/cli-common'),
|
||||
targetPaths: {
|
||||
get dir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
get rootDir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
resolve: (...args: string[]) => mockDir.resolve(...args),
|
||||
resolveRoot: (...args: string[]) => mockDir.resolve(...args),
|
||||
},
|
||||
|
||||
@@ -35,7 +35,7 @@ type Options = {
|
||||
};
|
||||
|
||||
export async function loadCliConfig(options: Options) {
|
||||
const targetDir = options.targetDir ?? targetPaths.resolve();
|
||||
const targetDir = options.targetDir ?? targetPaths.dir;
|
||||
|
||||
// Consider all packages in the monorepo when loading in config
|
||||
const { packages } = await getPackages(targetDir);
|
||||
@@ -74,7 +74,7 @@ export async function loadCliConfig(options: Options) {
|
||||
? async name => process.env[name] || 'x'
|
||||
: undefined,
|
||||
watch: Boolean(options.watch),
|
||||
rootDir: targetPaths.resolveRoot(),
|
||||
rootDir: targetPaths.rootDir,
|
||||
argv: options.args.flatMap(t => ['--config', resolvePath(targetDir, t)]),
|
||||
});
|
||||
|
||||
|
||||
@@ -84,7 +84,7 @@ export default async (options: InfoOptions) => {
|
||||
|
||||
const lockfilePath = targetPaths.resolveRoot('yarn.lock');
|
||||
const lockfile = await Lockfile.load(lockfilePath);
|
||||
const targetPath = targetPaths.resolveRoot();
|
||||
const targetPath = targetPaths.rootDir;
|
||||
|
||||
// Get workspace package names and their versions
|
||||
const workspacePackages = new Map<string, string>();
|
||||
|
||||
@@ -22,7 +22,7 @@ import { ESLint } from 'eslint';
|
||||
|
||||
export default async (directories: string[], opts: OptionValues) => {
|
||||
const eslint = new ESLint({
|
||||
cwd: targetPaths.resolve(),
|
||||
cwd: targetPaths.dir,
|
||||
fix: opts.fix,
|
||||
extensions: ['js', 'jsx', 'ts', 'tsx', 'mjs', 'cjs'],
|
||||
});
|
||||
@@ -48,7 +48,7 @@ export default async (directories: string[], opts: OptionValues) => {
|
||||
|
||||
// This formatter uses the cwd to format file paths, so let's have that happen from the root instead
|
||||
if (opts.format === 'eslint-formatter-friendly') {
|
||||
process.chdir(targetPaths.resolveRoot());
|
||||
process.chdir(targetPaths.rootDir);
|
||||
}
|
||||
|
||||
const resultText = await formatter.format(results);
|
||||
|
||||
@@ -63,7 +63,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
|
||||
|
||||
// This formatter uses the cwd to format file paths, so let's have that happen from the root instead
|
||||
if (opts.format === 'eslint-formatter-friendly') {
|
||||
process.chdir(targetPaths.resolveRoot());
|
||||
process.chdir(targetPaths.rootDir);
|
||||
}
|
||||
|
||||
// Make sure lint output is colored unless the user explicitly disabled it
|
||||
@@ -78,7 +78,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
|
||||
const lintOptions = parseLintScript(pkg.packageJson.scripts?.lint);
|
||||
const base = {
|
||||
fullDir: pkg.dir,
|
||||
relativeDir: relativePath(targetPaths.resolveRoot(), pkg.dir),
|
||||
relativeDir: relativePath(targetPaths.rootDir, pkg.dir),
|
||||
lintOptions,
|
||||
parentHash: undefined,
|
||||
};
|
||||
@@ -114,7 +114,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
|
||||
shouldCache: Boolean(cacheContext),
|
||||
maxWarnings: opts.maxWarnings ?? -1,
|
||||
successCache: cacheContext?.entries,
|
||||
rootDir: targetPaths.resolveRoot(),
|
||||
rootDir: targetPaths.rootDir,
|
||||
},
|
||||
workerFactory: async ({
|
||||
fix,
|
||||
|
||||
@@ -26,16 +26,16 @@ import { createTypeDistProject } from '../../../../lib/typeDistProject';
|
||||
|
||||
export const pre = async () => {
|
||||
publishPreflightCheck({
|
||||
dir: targetPaths.resolve(),
|
||||
dir: targetPaths.dir,
|
||||
packageJson: await fs.readJson(targetPaths.resolve('package.json')),
|
||||
});
|
||||
|
||||
await productionPack({
|
||||
packageDir: targetPaths.resolve(),
|
||||
packageDir: targetPaths.dir,
|
||||
featureDetectionProject: await createTypeDistProject(),
|
||||
});
|
||||
};
|
||||
|
||||
export const post = async () => {
|
||||
await revertProductionPack(targetPaths.resolve());
|
||||
await revertProductionPack(targetPaths.dir);
|
||||
};
|
||||
|
||||
@@ -230,7 +230,7 @@ export function createRepositoryFieldFixer() {
|
||||
return (pkg: FixablePackage) => {
|
||||
const expectedPath = posix.join(
|
||||
rootDir,
|
||||
relativePath(targetPaths.resolveRoot(), pkg.dir),
|
||||
relativePath(targetPaths.rootDir, pkg.dir),
|
||||
);
|
||||
const repoField = pkg.packageJson.repository;
|
||||
|
||||
@@ -319,7 +319,7 @@ export function fixPluginId(pkg: FixablePackage) {
|
||||
role === 'backend-plugin-module')
|
||||
) {
|
||||
const path = relativePath(
|
||||
targetPaths.resolveRoot(),
|
||||
targetPaths.rootDir,
|
||||
resolvePath(pkg.dir, 'package.json'),
|
||||
);
|
||||
const msg = `Failed to guess plugin ID for "${pkg.packageJson.name}", please set the 'backstage.pluginId' field manually in "${path}"`;
|
||||
@@ -415,7 +415,7 @@ export function fixPluginPackages(
|
||||
return;
|
||||
}
|
||||
const path = relativePath(
|
||||
targetPaths.resolveRoot(),
|
||||
targetPaths.rootDir,
|
||||
resolvePath(pkg.dir, 'package.json'),
|
||||
);
|
||||
const suggestedRole =
|
||||
@@ -464,7 +464,7 @@ export function fixPeerModules(pkg: FixablePackage) {
|
||||
}
|
||||
|
||||
const packagePath = relativePath(
|
||||
targetPaths.resolveRoot(),
|
||||
targetPaths.rootDir,
|
||||
resolvePath(pkg.dir, 'package.json'),
|
||||
);
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ export async function command(opts: OptionValues) {
|
||||
const packages = await PackageGraph.listTargetPackages();
|
||||
|
||||
const eslint = new ESLint({
|
||||
cwd: targetPaths.resolve(),
|
||||
cwd: targetPaths.dir,
|
||||
overrideConfig: {
|
||||
plugins: ['deprecation'],
|
||||
rules: {
|
||||
@@ -53,7 +53,7 @@ export async function command(opts: OptionValues) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const path = relativePath(targetPaths.resolveRoot(), result.filePath);
|
||||
const path = relativePath(targetPaths.rootDir, result.filePath);
|
||||
deprecations.push({
|
||||
path,
|
||||
message: message.message,
|
||||
|
||||
@@ -22,7 +22,7 @@ import { targetPaths } from '@backstage/cli-common';
|
||||
|
||||
|
||||
export default async () => {
|
||||
const { packages } = await getPackages(targetPaths.resolve());
|
||||
const { packages } = await getPackages(targetPaths.dir);
|
||||
|
||||
await Promise.all(
|
||||
packages.map(async ({ dir, packageJson: pkg }) => {
|
||||
|
||||
@@ -65,6 +65,12 @@ jest.mock('@backstage/cli-common', () => {
|
||||
return {
|
||||
...actual,
|
||||
targetPaths: {
|
||||
get dir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
get rootDir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
resolve: (...args: string[]) => mockDir.resolve(...args),
|
||||
resolveRoot: (...args: string[]) => mockDir.resolve(...args),
|
||||
},
|
||||
|
||||
@@ -141,7 +141,7 @@ export default async (opts: OptionValues) => {
|
||||
}
|
||||
|
||||
// First we discover all Backstage dependencies within our own repo
|
||||
const dependencyMap = await mapDependencies(targetPaths.resolve(), pattern);
|
||||
const dependencyMap = await mapDependencies(targetPaths.dir, pattern);
|
||||
|
||||
// Next check with the package registry to see which dependency ranges we need to bump
|
||||
const versionBumps = new Map<string, PkgVersionInfo[]>();
|
||||
|
||||
@@ -38,6 +38,12 @@ jest.mock('@backstage/cli-common', () => {
|
||||
return {
|
||||
...actual,
|
||||
targetPaths: {
|
||||
get dir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
get rootDir() {
|
||||
return mockDir.path;
|
||||
},
|
||||
resolve: (...args: string[]) => mockDir.resolve(...args),
|
||||
resolveRoot: (...args: string[]) => mockDir.resolve(...args),
|
||||
},
|
||||
|
||||
@@ -83,7 +83,7 @@ export async function addCodeownersEntry(
|
||||
|
||||
let filePath = codeownersFilePath;
|
||||
if (!filePath) {
|
||||
filePath = await getCodeownersFilePath(targetPaths.resolveRoot());
|
||||
filePath = await getCodeownersFilePath(targetPaths.rootDir);
|
||||
if (!filePath) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -39,7 +39,7 @@ export async function collectPortableTemplateInput(
|
||||
): Promise<PortableTemplateInput> {
|
||||
const { config, template, prefilledParams } = options;
|
||||
|
||||
const codeOwnersFilePath = await getCodeownersFilePath(targetPaths.resolveRoot());
|
||||
const codeOwnersFilePath = await getCodeownersFilePath(targetPaths.rootDir);
|
||||
|
||||
const prompts = getPromptsForRole(template.role);
|
||||
|
||||
|
||||
@@ -68,7 +68,7 @@ interface TestGlobal extends Global {
|
||||
async function readPackageTreeHashes(graph: PackageGraph) {
|
||||
const pkgs = Array.from(graph.values()).map(pkg => ({
|
||||
...pkg,
|
||||
path: relativePath(targetPaths.resolveRoot(), pkg.dir),
|
||||
path: relativePath(targetPaths.rootDir, pkg.dir),
|
||||
}));
|
||||
const output = await runOutput([
|
||||
'git',
|
||||
|
||||
Reference in New Issue
Block a user