paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 23 Packages Projects Releases Wiki Activity

Address review feedback from freben

Browse Source 
- Use cli-defaults instead of listing individual CLI modules in
  create-app template and root package.json
- Move nodeTransform config files from cli-module-build to cli-node
  to avoid cross-module direct imports
- Rename cli-module-create-github-app to cli-module-github
- Start createCliModule init chain with Promise.resolve()
- Deduplicate exitWithError in runCliModule.ts
- Extract shared isCommandNodeHidden to @internal/cli
- Add explanatory comment for fromArray deduplication field
- Restore error for cli role packages missing bin in runCliExtraction

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Made-with: Cursor
This commit is contained in:
Patrik Oldsberg
2026-03-16 12:55:22 +01:00
parent 26eab3bf83
commit 4d081452b1
47 changed files with 147 additions and 185 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli-module-build/bin/backstage-cli-module-build
+1 -1
View File
@@ -28,6 +28,6 @@ if (!isLocal) {
const pkg = require('../package.json');
runCliModule({ module: cliModule, name: pkg.name, version: pkg.version });
} else {
require('@backstage/cli/config/nodeTransform.cjs');
require('@backstage/cli-node/config/nodeTransform.cjs');
require('../src/cli');
}
packages/cli-module-build/config/nodeTransform.cjs
-87
View File
@@ -1,87 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
const { pathToFileURL } = require('node:url');
const { transformSync } = require('@swc/core');
const { addHook } = require('pirates');
const { Module } = require('node:module');
// This hooks into module resolution and overrides imports of packages that
// exist in the linked workspace to instead be resolved from the linked workspace.
if (process.env.BACKSTAGE_CLI_LINKED_WORKSPACE) {
const { join: joinPath } = require('node:path');
const { getPackagesSync } = require('@manypkg/get-packages');
const { packages: linkedPackages, root: linkedRoot } = getPackagesSync(
process.env.BACKSTAGE_CLI_LINKED_WORKSPACE,
);
// Matches all packages in the linked workspaces, as well as sub-path exports from them
const replacementRegex = new RegExp(
`^(?:${linkedPackages
.map(pkg => pkg.packageJson.name)
.join('|')})(?:/.*)?$`,
);
const origLoad = Module._load;
Module._load = function requireHook(request, parent) {
if (!replacementRegex.test(request)) {
return origLoad.call(this, request, parent);
}
// The package import that we're overriding will always existing in the root
// node_modules of the linked workspace, so it's enough to override the
// parent paths with that single entry
return origLoad.call(this, request, {
...parent,
paths: [joinPath(linkedRoot.dir, 'node_modules')],
});
};
}
addHook(
(code, filename) => {
const transformed = transformSync(code, {
filename,
sourceMaps: 'inline',
module: {
type: 'commonjs',
ignoreDynamic: true,
},
jsc: {
target: 'es2023',
parser: {
syntax: 'typescript',
},
},
});
process.send?.({ type: 'watch', path: filename });
return transformed.code;
},
{ extensions: ['.ts', '.cts'], ignoreNodeModules: true },
);
addHook(
(code, filename) => {
process.send?.({ type: 'watch', path: filename });
return code;
},
{ extensions: ['.js', '.cjs'], ignoreNodeModules: true },
);
// Register module hooks, used by "type": "module" in package.json, .mjs and
// .mts files, as well as dynamic import(...)s, although dynamic imports will be
// handled be the CommonJS hooks in this file if what it points to is CommonJS.
Module.register('./nodeTransformHooks.mjs', pathToFileURL(__filename));
packages/cli-module-build/config/nodeTransformHooks.mjs
-294
View File
@@ -1,294 +0,0 @@
/*
* Copyright 2024 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { dirname, extname, resolve as resolvePath } from 'node:path';
import { fileURLToPath } from 'node:url';
import { transformFile } from '@swc/core';
import { isBuiltin } from 'node:module';
import { readFile } from 'node:fs/promises';
import { existsSync } from 'node:fs';
// @ts-check
// No explicit file extension, no type in package.json
const DEFAULT_MODULE_FORMAT = 'commonjs';
// Source file extensions to look for when using bundle resolution strategy
const SRC_EXTS = ['.ts', '.js'];
const TS_EXTS = ['.ts', '.mts', '.cts'];
const moduleTypeTable = {
'.mjs': 'module',
'.mts': 'module',
'.cjs': 'commonjs',
'.cts': 'commonjs',
'.ts': undefined,
'.js': undefined,
};
/** @type {import('module').ResolveHook} */
export async function resolve(specifier, context, nextResolve) {
// Built-in modules are handled by the default resolver
if (isBuiltin(specifier)) {
return nextResolve(specifier, context);
}
const ext = extname(specifier);
// Unless there's an explicit import attribute, JSON files are loaded with our custom loader that's defined below.
if (ext === '.json' && !context.importAttributes?.type) {
const jsonResult = await nextResolve(specifier, context);
return {
...jsonResult,
format: 'commonjs',
importAttributes: { type: 'json' },
};
}
// Anything else with an explicit extension is handled by the default
// resolver, except that we help determine the module type where needed.
if (ext !== '') {
return withDetectedModuleType(await nextResolve(specifier, context));
}
// Other external modules are handled by the default resolver, but again we
// help determine the module type where needed.
if (!specifier.startsWith('.')) {
return withDetectedModuleType(await nextResolve(specifier, context));
}
// The rest of this function handles the case of resolving imports that do not
// specify any extension and might point to a directory with an `index.*`
// file. We resolve those using the same logic as most JS bundlers would, with
// the addition of checking if there's an explicit module format listed in the
// closest `package.json` file.
//
// We use a bundle resolution strategy in order to keep code consistent across
// Backstage codebases that contains code both for Web and Node.js, and to
// support packages with common code that can be used in both environments.
try {
// This is expected to throw, but in the event that this module specifier is
// supported we prefer to use the default resolver.
return await nextResolve(specifier, context);
} catch (error) {
if (error.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
const spec = `${specifier}${specifier.endsWith('/') ? '' : '/'}index`;
const resolved = await resolveWithoutExt(spec, context, nextResolve);
if (resolved) {
return withDetectedModuleType(resolved);
}
} else if (error.code === 'ERR_MODULE_NOT_FOUND') {
const resolved = await resolveWithoutExt(specifier, context, nextResolve);
if (resolved) {
return withDetectedModuleType(resolved);
}
}
// Unexpected error or no resolution found
throw error;
}
}
/**
* Populates the `format` field in the resolved object based on the closest `package.json` file.
*
* @param {import('module').ResolveFnOutput} resolved
* @returns {Promise<import('module').ResolveFnOutput>}
*/
async function withDetectedModuleType(resolved) {
// Already has an explicit format
if (resolved.format) {
return resolved;
}
// Happens in Node.js v22 when there's a package.json without an explicit "type" field. Use the default.
if (resolved.format === null) {
return { ...resolved, format: DEFAULT_MODULE_FORMAT };
}
const ext = extname(resolved.url);
const explicitFormat = moduleTypeTable[ext];
if (explicitFormat) {
return {
...resolved,
format: explicitFormat,
};
}
// Under normal circumstances .js files should reliably have a format and so
// we should only reach this point for .ts files. However, if additional
// custom loaders are being used the format may not be detected for .js files
// either. As such we don't restrict the file format at this point.
// TODO(Rugvip): Does this need caching? kept it simple for now but worth exploring
const packageJsonPath = await findPackageJSON(fileURLToPath(resolved.url));
if (!packageJsonPath) {
return resolved;
}
const packageJson = JSON.parse(await readFile(packageJsonPath, 'utf8'));
return {
...resolved,
format: packageJson.type ?? DEFAULT_MODULE_FORMAT,
};
}
/**
* Find the closest package.json file from the given path.
*
* TODO(Rugvip): This can be replaced with the Node.js built-in with the same name once it is stable.
* @param {string} startPath
* @returns {Promise<string | undefined>}
*/
async function findPackageJSON(startPath) {
let path = startPath;
// Some confidence check to avoid infinite loop
for (let i = 0; i < 1000; i++) {
const packagePath = resolvePath(path, 'package.json');
if (existsSync(packagePath)) {
return packagePath;
}
const newPath = dirname(path);
if (newPath === path) {
return undefined;
}
path = newPath;
}
throw new Error(
`Iteration limit reached when searching for package.json at ${startPath}`,
);
}
/** @type {import('module').ResolveHook} */
async function resolveWithoutExt(specifier, context, nextResolve) {
for (const tryExt of SRC_EXTS) {
try {
const resolved = await nextResolve(specifier + tryExt, {
...context,
format: 'commonjs',
});
return {
...resolved,
format: moduleTypeTable[tryExt] ?? resolved.format,
};
} catch {
/* ignore */
}
}
return undefined;
}
/** @type {import('module').LoadHook} */
export async function load(url, context, nextLoad) {
// Non-file URLs are handled by the default loader
if (!url.startsWith('file://')) {
return nextLoad(url, context);
}
// JSON files loaded as CommonJS are handled by this custom loader, because
// the default one doesn't work. For JSON loading to work we'd need the
// synchronous hooks that aren't supported yet, or avoid using the CommonJS
// compatibility.
if (
context.format === 'commonjs' &&
context.importAttributes?.type === 'json'
) {
try {
// TODO(Rugvip): Make sure this is valid JSON
const content = await readFile(fileURLToPath(url), 'utf8');
return {
source: `module.exports = (${content})`,
format: 'commonjs',
shortCircuit: true,
};
} catch {
// Let the default loader generate the error
return nextLoad(url, context);
}
}
const ext = extname(url);
// Non-TS files are handled by the default loader
if (!TS_EXTS.includes(ext)) {
return nextLoad(url, context);
}
const format = context.format ?? DEFAULT_MODULE_FORMAT;
// We have two choices at this point, we can either transform CommonJS files
// and return the transformed source code, or let the default loader handle
// them. If we transform them ourselves we will enter CommonJS compatibility
// mode in the new module system in Node.js, this effectively means all
// CommonJS loaded via `require` calls from this point will all be treated as
// if it was loaded via `import` calls from modules.
//
// The CommonJS compatibility layer will try to identify named exports and
// make them available directly, which is convenient as it avoids things like
// `import(...).then(m => m.default.foo)`, allowing you to instead write
// `import(...).then(m => m.foo)`. The compatibility layer doesn't always work
// all that well though, and can lead to module loading issues in many cases,
// especially for older code.
// This `if` block opts-out of using CommonJS compatibility mode by default,
// and instead leaves it to our existing loader to transform CommonJS. We do
// however use compatibility mode for the more explicit .cts file extension,
// allows for a way to opt-in to the new behavior.
//
// TODO(Rugvip): Once the synchronous hooks API is available for us to use, we might be able to adopt that instead
if (format === 'commonjs' && ext !== '.cts') {
return nextLoad(url, { ...context, format });
}
// If the Node.js version we're running supports TypeScript, i.e. type
// stripping, we hand over to the default loader. This is done for all cases
// except if we're loading a .ts file that's been resolved to CommonJS format.
// This is because these files aren't actually CommonJS in the Backstage build
// system, and need to be transformed to CommonJS.
if (
format === 'module-typescript' ||
(format === 'module-commonjs' && ext !== '.ts')
) {
return nextLoad(url, { ...context, format });
}
const transformed = await transformFile(fileURLToPath(url), {
sourceMaps: 'inline',
module: {
type: format === 'module' ? 'es6' : 'commonjs',
ignoreDynamic: true,
// This helps the Node.js CommonJS compat layer identify named exports.
exportInteropAnnotation: true,
},
jsc: {
target: 'es2023',
parser: {
syntax: 'typescript',
},
},
});
return {
...context,
shortCircuit: true,
source: transformed.code,
format,
responseURL: url,
};
}
packages/cli-module-build/package.json
-1
View File
@@ -72,7 +72,6 @@
"node-stdlib-browser": "^1.3.1",
"npm-packlist": "^5.0.0",
"p-queue": "^6.6.2",
"pirates": "^4.0.6",
"postcss": "^8.1.0",
"postcss-import": "^16.1.0",
"process": "^0.11.10",
packages/cli-module-build/src/lib/runner/runBackend.ts
+3 -4
View File
@@ -21,16 +21,15 @@ import { IpcServer, ServerDataStore } from '../ipc';
import debounce from 'lodash/debounce';
import { fileURLToPath } from 'node:url';
import { isAbsolute as isAbsolutePath } from 'node:path';
import { findOwnPaths, targetPaths } from '@backstage/cli-common';
import { targetPaths } from '@backstage/cli-common';
import spawn from 'cross-spawn';
const loaderArgs = [
'--enable-source-maps',
'--require',
/* eslint-disable-next-line no-restricted-syntax */
findOwnPaths(__dirname).resolve('config/nodeTransform.cjs'),
// TODO: Support modules, although there's currently no way to load them since import() is transpiled tp require()
require.resolve('@backstage/cli-node/config/nodeTransform.cjs'),
// TODO: Support modules, although there's currently no way to load them since import() is transpiled to require()
];
export type RunBackendOptions = {
packages/cli-module-build/src/tests/transforms/transforms.test.ts
+1 -3
View File
@@ -16,7 +16,6 @@
import { execFileSync } from 'node:child_process';
import { resolve as resolvePath } from 'node:path';
import { findOwnPaths } from '@backstage/cli-common';
import { Output, buildPackage } from '../../lib/builder';
const exportValues = {
@@ -56,8 +55,7 @@ function loadFixture(fixture: string) {
'node',
[
'--import',
/* eslint-disable-next-line no-restricted-syntax */
findOwnPaths(__dirname).resolve('config/nodeTransform.cjs'),
require.resolve('@backstage/cli-node/config/nodeTransform.cjs'),
resolvePath(__dirname, `__fixtures__/${fixture}`),
],
{ encoding: 'utf8' },