fix(schematics): ship shared util chunks and load the build as CommonJS (#DS-5799)

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",

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,29 +32,63 @@ 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'),
         },
         external: (dependency) =>
             !(dependency.startsWith('./') || dependency.startsWith('../') || dependency.startsWith(__dirname)),
         plugins: [
             clean(),
+            rejectTestImports(),
             replace({
                 preventAssignment: true,
                 'VERSIONS.ANGULAR_CDK': version(pkg.dependencies['@angular/cdk']),

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;
+}

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,17 +71,20 @@ 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);
     await ensureDirectoryExistence(ngAddPath);
     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) : [];
 

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<string, string[]> = { 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[] = [];