diff --git a/package.json b/package.json index 0199b0da9a..92580ece64 100644 --- a/package.json +++ b/package.json @@ -188,7 +188,7 @@ "build:docs-pages": "ts-node --project ./tools/docs-pages/tsconfig.json ./tools/docs-pages/generate-docs-pages.ts", "build:package-docs-content": "ts-node --project ./tools/package-docs-content/tsconfig.json ./tools/package-docs-content/package-docs-content.ts", "styles:build-all": "ts-node --project tools/scss-compiler/tsconfig.json tools/scss-compiler/scss-bundle.ts", - "build:schematics": "rimraf packages/schematics/dist && rollup --failAfterWarnings -c packages/schematics/rollup.config.js && node ./packages/schematics/scripts/copy-meta-to-dist.js", + "build:schematics": "rimraf packages/schematics/dist && rollup --failAfterWarnings -c packages/schematics/rollup.config.js && node ./packages/schematics/scripts/copy-meta-to-dist.js && node ./packages/schematics/scripts/check-dist.mjs", "build:mcp": "yarn run build:mcp:index && tsc -p packages/mcp/tsconfig.json && node packages/mcp/scripts/copy-to-dist.cjs", "build:mcp:index": "tsx packages/mcp/src/indexer/build-index.ts", "build:mcp:data": "node packages/mcp/scripts/build-data.cjs", diff --git a/packages/schematics/rollup.config.js b/packages/schematics/rollup.config.js index 3dba3570f7..adc4aa85fd 100644 --- a/packages/schematics/rollup.config.js +++ b/packages/schematics/rollup.config.js @@ -2,7 +2,7 @@ const typescript = require('@rollup/plugin-typescript'); const replace = require('@rollup/plugin-replace'); const path = require('path'); -const { promises: fs, statSync } = require('fs'); +const { promises: fs, readdirSync, statSync } = require('fs'); const pkg = require('../../package.json'); const { getMigrations } = require('./src/utils/migrations'); @@ -32,22 +32,55 @@ const getMigrationInputs = () => { }, {}); }; +/** + * Every util module is an entry of its own, so it ships under a stable name rather than inside a hashed chunk. + * `.d.ts` and `.spec.ts` files do not match; `testing.ts` holds the spec helpers. + */ +const getUtilInputs = () => { + const utilsDir = path.join(__dirname, 'src/utils'); + + return readdirSync(utilsDir) + .filter((file) => /^[\w-]+\.ts$/.test(file) && file !== 'testing.ts') + .reduce((res, file) => { + res[`utils/${path.basename(file, '.ts')}`] = path.join(utilsDir, file); + + return res; + }, {}); +}; + +const TEST_IMPORT = /^vitest(\/|$)|\/testing$/; + +/** Fails the build when shipped code imports test tooling, e.g. a spec helper that became an entry. */ +const rejectTestImports = () => ({ + name: 'reject-test-imports', + generateBundle(_, bundle) { + for (const chunk of Object.values(bundle)) { + const testImport = + chunk.type === 'chunk' && + [...chunk.imports, ...chunk.dynamicImports].find((id) => TEST_IMPORT.test(id)); + + if (testImport) { + this.error(`${chunk.fileName} imports ${testImport}, which consumers do not have`); + } + } + } +}); + module.exports = [ { output: { dir: distDir, format: 'cjs', - exports: 'named' + exports: 'named', + // `.cjs` stays CommonJS inside @koobiq/components, which ng-packagr marks `"type": "module"`. + entryFileNames: '[name].cjs', + // A module shared by several entries without being one goes next to the utils, which + // copy-meta-to-dist.js ships as a whole. + chunkFileNames: 'utils/[name]-[hash].cjs' }, input: { 'ng-add/index': path.join(__dirname, 'src/ng-add/index.ts'), - 'utils/package-config': path.join(__dirname, 'src/utils/package-config.ts'), - 'utils/messages': path.join(__dirname, 'src/utils/messages.ts'), - 'utils/typescript': path.join(__dirname, 'src/utils/typescript.ts'), - 'utils/ast': path.join(__dirname, 'src/utils/ast.ts'), - 'utils/angular-parsing': path.join(__dirname, 'src/utils/angular-parsing.ts'), - 'utils/icon-migration': path.join(__dirname, 'src/utils/icon-migration.ts'), - 'utils/deprecated-path-migration': path.join(__dirname, 'src/utils/deprecated-path-migration.ts'), + ...getUtilInputs(), ...getMigrationInputs() //'ng-add/setup-project': path.join(__dirname, 'ng-add/setup-project.ts'), }, @@ -55,6 +88,7 @@ module.exports = [ !(dependency.startsWith('./') || dependency.startsWith('../') || dependency.startsWith(__dirname)), plugins: [ clean(), + rejectTestImports(), replace({ preventAssignment: true, 'VERSIONS.ANGULAR_CDK': version(pkg.dependencies['@angular/cdk']), diff --git a/packages/schematics/scripts/check-dist.mjs b/packages/schematics/scripts/check-dist.mjs new file mode 100644 index 0000000000..22829b3e33 --- /dev/null +++ b/packages/schematics/scripts/check-dist.mjs @@ -0,0 +1,153 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { createRequire, isBuiltin } from 'node:module'; +import { dirname, join, relative, resolve, sep } from 'node:path'; +import ts from 'typescript'; + +/** + * Checks dist/components/schematics the way `ng add` and `ng update` load it: every module is `.cjs`, every + * `require()` and `import()` names a file of the package or a package that consumers have and resolves, and + * every schematic of both collections has a factory that loads and a schema that parses. Whether npm packs + * every file is checked by `check-npm-resolution`, which already packs the package. + */ + +const root = resolve(import.meta.dirname, '../../..'); +const packagePath = join(root, 'dist/components'); +const schematicsPath = join(packagePath, 'schematics'); +const require = createRequire(import.meta.url); + +const failures = []; + +const readJson = (file) => JSON.parse(readFileSync(file, 'utf8')); + +const toPackagePath = (file) => relative(packagePath, file).split(sep).join('/'); + +const componentsManifest = readJson(join(root, 'packages/components/package.json')); + +/** The non-optional peers of @koobiq/components and what the Angular CLI brings into every workspace. */ +const consumerPackages = new Set([ + ...Object.keys(componentsManifest.dependencies ?? {}), + ...Object.keys(componentsManifest.peerDependencies ?? {}).filter( + (name) => !componentsManifest.peerDependenciesMeta?.[name]?.optional + ), + '@angular-devkit/schematics', + '@angular/compiler', + '@schematics/angular', + 'typescript' +]); + +const packageName = (specifier) => specifier.split('/', specifier.startsWith('@') ? 2 : 1).join('/'); + +/** The string specifiers of `require()` and `import()` calls, read from the syntax tree rather than the text. */ +const collectImports = (file) => { + const imports = []; + const visit = (node) => { + if (ts.isCallExpression(node) && node.arguments.length > 0 && ts.isStringLiteralLike(node.arguments[0])) { + const specifier = node.arguments[0].text; + + if (node.expression.kind === ts.SyntaxKind.ImportKeyword) { + imports.push({ kind: 'import', specifier }); + } else if (ts.isIdentifier(node.expression) && node.expression.text === 'require') { + imports.push({ kind: 'require', specifier }); + } + } + + ts.forEachChild(node, visit); + }; + + visit(ts.createSourceFile(file, readFileSync(file, 'utf8'), ts.ScriptTarget.Latest, false, ts.ScriptKind.JS)); + + return imports; +}; + +// `import()` resolves with the ESM conditions, which Node applies to a bare specifier only relative to the +// calling module. This script resolves it from here: it shares the repository's node_modules with dist/. +const resolveImport = ({ kind, specifier }, file) => + kind === 'import' && !specifier.startsWith('.') + ? import.meta.resolve(specifier) + : createRequire(file).resolve(specifier); + +const shippedFiles = readdirSync(schematicsPath, { recursive: true }) + .map((file) => join(schematicsPath, file)) + .filter((file) => statSync(file).isFile()); + +for (const file of shippedFiles.filter((file) => file.endsWith('.js'))) { + failures.push(`${toPackagePath(file)}: loads as an ES module, since ng-packagr marks the package "type": "module"`); +} + +for (const file of shippedFiles.filter((file) => file.endsWith('.cjs'))) { + for (const entry of collectImports(file)) { + const { specifier } = entry; + + if (!specifier.startsWith('.') && !isBuiltin(specifier) && !consumerPackages.has(packageName(specifier))) { + failures.push(`${toPackagePath(file)}: '${specifier}' is not a package that consumers have`); + continue; + } + + try { + resolveImport(entry, file); + } catch { + failures.push(`${toPackagePath(file)}: cannot resolve '${specifier}'`); + } + } +} + +const manifestPath = join(packagePath, 'package.json'); + +// Built on their own, the schematics have no manifest from ng-packagr; its fields point at the same files. +const manifest = statSync(manifestPath, { throwIfNoEntry: false }) + ? readJson(manifestPath) + : { schematics: './schematics/collection.json', 'ng-update': { migrations: './schematics/migrations.json' } }; + +for (const [field, collectionPath] of [ + ['schematics', manifest.schematics], + ['ng-update.migrations', manifest['ng-update']?.migrations] +]) { + if (!collectionPath) { + failures.push(`package.json: no ${field}`); + continue; + } + + const collectionFile = join(packagePath, collectionPath); + const collectionDir = dirname(collectionFile); + let schematics; + + try { + ({ schematics } = readJson(collectionFile)); + } catch (error) { + failures.push(`${field} ${collectionPath}: ${error.message.split('\n')[0]}`); + continue; + } + + for (const [name, { factory, schema }] of Object.entries(schematics)) { + const where = `${toPackagePath(collectionFile)}: ${name}`; + + if (typeof factory !== 'string') { + failures.push(`${where} has no factory`); + continue; + } + + const [modulePath, exportName = 'default'] = factory.split('#'); + + try { + // A plain require(), as `ExportStringRef` of @angular-devkit/schematics loads a factory. + if (typeof require(join(collectionDir, modulePath))[exportName] !== 'function') { + failures.push(`${where}: the factory ${factory} is not a function`); + } + } catch (error) { + failures.push(`${where}: the factory ${factory} fails to load: ${error.message.split('\n')[0]}`); + } + + if (schema) { + try { + readJson(join(collectionDir, schema)); + } catch (error) { + failures.push(`${where}: the schema ${schema} cannot be read: ${error.message.split('\n')[0]}`); + } + } + } +} + +if (failures.length > 0) { + console.error([`${failures.length} problem(s) in dist/components/schematics:`, ...failures].join('\n ')); + process.exitCode = 1; +} diff --git a/packages/schematics/scripts/copy-meta-to-dist.js b/packages/schematics/scripts/copy-meta-to-dist.js index e1583a8a1a..762869299b 100644 --- a/packages/schematics/scripts/copy-meta-to-dist.js +++ b/packages/schematics/scripts/copy-meta-to-dist.js @@ -1,4 +1,4 @@ -const { access, copyFile, mkdir, readdir } = require('fs/promises'); +const { access, copyFile, mkdir, readdir, readFile, rm, writeFile } = require('fs/promises'); const { resolve, join } = require('path'); const { getMigrations } = require('../src/utils/migrations'); const { statSync } = require('fs'); @@ -31,6 +31,27 @@ const copyFileWrapper = async (src, dest) => { } }; +/** + * Copies collection.json or migrations.json with every factory pointing at its `.cjs` module. The sources + * keep the factories extension-less for the specs, which load the `.ts` files, and `require.resolve()` + * does not try `.cjs` on its own. + */ +const copyCollection = async (name, destDir) => { + const src = resolvePath(`../src/${name}`); + + try { + const collection = JSON.parse(await readFile(src, 'utf8')); + + for (const schematic of Object.values(collection.schematics)) { + schematic.factory = schematic.factory.replace(/^[^#]+/, '$&.cjs'); + } + + await writeFile(join(destDir, name), `${JSON.stringify(collection, null, 4)}\n`); + } catch (error) { + failures.push(`Failed to copy ${src}: ${error.message}`); + } +}; + /** Copies the meta files of one migration, naming the migration instead of reporting a bare ENOENT path. */ const copyMigrationMeta = async (migration, migrationPath) => { for (const file of MIGRATION_META_FILES) { @@ -50,6 +71,9 @@ const init = async () => { const ngAddPath = join(schematicsPath, 'ng-add'); const utilsPath = join(schematicsPath, 'utils'); + // Without a clean start a renamed or removed migration would stay in the package. + await rm(schematicsPath, { recursive: true, force: true }); + // Ensure directories exist await ensureDirectoryExistence(distCLIPath); await ensureDirectoryExistence(schematicsPath); @@ -57,10 +81,10 @@ const init = async () => { await ensureDirectoryExistence(utilsPath); // Copy files - await copyFileWrapper(resolvePath('../dist/ng-add/index.js'), join(ngAddPath, 'index.js')); + await copyFileWrapper(resolvePath('../dist/ng-add/index.cjs'), join(ngAddPath, 'index.cjs')); await copyFileWrapper(resolvePath('../src/ng-add/schema.json'), join(ngAddPath, 'schema.json')); - await copyFileWrapper(resolvePath('../src/collection.json'), join(schematicsPath, 'collection.json')); - await copyFileWrapper(resolvePath('../src/migrations.json'), join(schematicsPath, 'migrations.json')); + await copyCollection('collection.json', schematicsPath); + await copyCollection('migrations.json', schematicsPath); for (const migration of getMigrations()) { const migrationPath = join(schematicsPath, 'migrations', migration); @@ -69,12 +93,15 @@ const init = async () => { await copyMigrationMeta(migration, migrationPath); - await copyFileWrapper(resolvePath(`../dist/migrations/${migration}/index.js`), join(migrationPath, 'index.js')); - const optionalMigrationData = resolvePath(`../dist/migrations/${migration}/data.js`); + await copyFileWrapper( + resolvePath(`../dist/migrations/${migration}/index.cjs`), + join(migrationPath, 'index.cjs') + ); + const optionalMigrationData = resolvePath(`../dist/migrations/${migration}/data.cjs`); const fileExists = statSync(optionalMigrationData, { throwIfNoEntry: false }); if (fileExists) { - await copyFileWrapper(optionalMigrationData, join(migrationPath, 'data.js')); + await copyFileWrapper(optionalMigrationData, join(migrationPath, 'data.cjs')); } } @@ -87,8 +114,9 @@ const init = async () => { join(schematicsPath, 'migrations', 'new-icons-pack', 'replacement.json') ); - // Copy every compiled util chunk rather than a hand-maintained list, so a new util module - // (given a stable rollup input name under `utils/`) is shipped automatically. + // Copy every compiled util rather than a hand-maintained list: rollup.config.js gives each + // `src/utils` module an entry under `utils/` and puts the shared chunks there too, so a new one + // is shipped automatically. const distUtilsPath = resolvePath('../dist/utils'); const utilFiles = statSync(distUtilsPath, { throwIfNoEntry: false }) ? await readdir(distUtilsPath) : []; diff --git a/tools/check-npm-resolution/index.ts b/tools/check-npm-resolution/index.ts index 3e285147ef..38dc90bf3a 100644 --- a/tools/check-npm-resolution/index.ts +++ b/tools/check-npm-resolution/index.ts @@ -7,16 +7,17 @@ * against the project shapes consumers actually have. * * The fixtures are the installation paths the documentation supports: `ng add`, the manual install - * line, and an application that already carries the Angular packages. + * line, and an application that already carries the Angular packages. Packing also fails when the + * tarball leaves out a file of the schematics. * * Runs on ubuntu in CI. Node >= 20.12 refuses to spawn `npm.cmd` without a shell, so on Windows the * calls go through one — hence the quoting in `npm()`. */ import { spawnSync } from 'child_process'; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'fs'; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, statSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; -import { join } from 'path'; +import { join, sep } from 'path'; interface PackageJson { version?: string; @@ -189,16 +190,36 @@ const workDir = mkdtempSync(join(tmpdir(), 'koobiq-npm-resolution-')); console.log(`Packing ${packagesToCheck.length} package(s) from dist/...`); +/** + * Directories every file of which has to reach the tarball: `ng add` and `ng update` load the schematics + * from it, and the `.npmignore` that ng-packagr writes drops files by pattern. + */ +const shippedWhole: Record = { components: ['schematics'] }; + const tarballs = packagesToCheck.map((name) => { - const packed = npm(['pack', join(distDir, name), '--pack-destination', workDir], projectRoot); + const packed = npm(['pack', join(distDir, name), '--pack-destination', workDir, '--json'], projectRoot); if (packed.status !== 0) { console.error(`❌ Could not pack ${name}:\n${packed.output}`); process.exit(1); } - // npm prints notices on stderr and the bare filename on stdout. - return join(workDir, packed.stdout.trim().split('\n').pop()!); + // npm prints notices on stderr and the JSON report on stdout. + const [{ filename, files }]: { filename: string; files: { path: string }[] }[] = JSON.parse(packed.stdout); + const packedFiles = new Set(files.map(({ path }) => path)); + + for (const dir of shippedWhole[name] ?? []) { + const left = readdirSync(join(distDir, name, dir), { recursive: true, encoding: 'utf-8' }) + .map((file) => `${dir}/${file.split(sep).join('/')}`) + .filter((file) => statSync(join(distDir, name, file)).isFile() && !packedFiles.has(file)); + + if (left.length > 0) { + console.error(`❌ npm pack leaves out of ${name}:\n${left.map((file) => ` ${file}`).join('\n')}`); + process.exit(1); + } + } + + return join(workDir, filename); }); const failures: string[] = [];