diff --git a/packages/eslint-plugin/src/rules/dependency-checks.spec.ts b/packages/eslint-plugin/src/rules/dependency-checks.spec.ts index 9f4c136715812..122b5c680b403 100644 --- a/packages/eslint-plugin/src/rules/dependency-checks.spec.ts +++ b/packages/eslint-plugin/src/rules/dependency-checks.spec.ts @@ -202,6 +202,75 @@ describe('Dependency checks (eslint)', () => { expect(failures.length).toEqual(0); }); + it('should exclude files that are ignored', () => { + const packageJson = { + name: '@mycompany/liba', + dependencies: {}, + }; + + const fileSys = { + './libs/liba/package.json': JSON.stringify(packageJson, null, 2), + './libs/liba/vite.config.ts': '', + './libs/liba/project.json': JSON.stringify( + { + name: 'liba', + targets: { + build: { + command: 'tsc -p tsconfig.lib.json', + }, + }, + }, + null, + 2 + ), + './nx.json': JSON.stringify({ + targetDefaults: { + build: { + inputs: [ + '{projectRoot}/**/*', + '!{projectRoot}/**/?(*.)+(spec|test).[jt]s?(x)?(.snap)', + ], + }, + }, + }), + './package.json': JSON.stringify(rootPackageJson, null, 2), + }; + vol.fromJSON(fileSys, '/root'); + + const failures = runRule( + { + ignoredFiles: ['{projectRoot}/vite.config.ts'], + }, + `/root/libs/liba/package.json`, + JSON.stringify(packageJson, null, 2), + { + nodes: { + liba: { + name: 'liba', + type: 'lib', + data: { + root: 'libs/liba', + targets: { + build: {}, + }, + }, + }, + }, + externalNodes, + dependencies: { + liba: [{ source: 'liba', target: 'npm:external1', type: 'static' }], + }, + }, + { + liba: [ + createFile(`libs/liba/vite.config.ts`, ['npm:external1']), + createFile(`libs/liba/package.json`, []), + ], + } + ); + expect(failures.length).toEqual(0); + }); + it('should report missing dependencies section and fix it', () => { const packageJson = { name: '@mycompany/liba', diff --git a/packages/eslint-plugin/src/rules/dependency-checks.ts b/packages/eslint-plugin/src/rules/dependency-checks.ts index 4c122b9d9ff64..5fc68dd1698a9 100644 --- a/packages/eslint-plugin/src/rules/dependency-checks.ts +++ b/packages/eslint-plugin/src/rules/dependency-checks.ts @@ -22,6 +22,7 @@ export type Options = [ checkVersionMismatches?: boolean; checkMissingPackageJson?: boolean; ignoredDependencies?: string[]; + ignoredFiles?: string[]; includeTransitiveDependencies?: boolean; } ]; @@ -49,6 +50,7 @@ export default createESLintRule({ properties: { buildTargets: [{ type: 'string' }], ignoredDependencies: [{ type: 'string' }], + ignoredFiles: [{ type: 'string' }], checkMissingDependencies: { type: 'boolean' }, checkObsoleteDependencies: { type: 'boolean' }, checkVersionMismatches: { type: 'boolean' }, @@ -71,6 +73,7 @@ export default createESLintRule({ checkObsoleteDependencies: true, checkVersionMismatches: true, ignoredDependencies: [], + ignoredFiles: [], includeTransitiveDependencies: false, }, ], @@ -80,6 +83,7 @@ export default createESLintRule({ { buildTargets, ignoredDependencies, + ignoredFiles, checkMissingDependencies, checkObsoleteDependencies, checkVersionMismatches, @@ -133,6 +137,7 @@ export default createESLintRule({ buildTarget, // TODO: What if child library has a build target different from the parent? { includeTransitiveDependencies, + ignoredFiles, } ); const expectedDependencyNames = Object.keys(npmDependencies); diff --git a/packages/js/src/generators/library/library.spec.ts b/packages/js/src/generators/library/library.spec.ts index 97a91e97cb737..8c91b438c0fe6 100644 --- a/packages/js/src/generators/library/library.spec.ts +++ b/packages/js/src/generators/library/library.spec.ts @@ -1137,6 +1137,20 @@ describe('lib', () => { executor: '@nx/vite:test', }); expect(tree.exists('libs/my-lib/vite.config.ts')).toBeTruthy(); + expect( + readJson(tree, 'libs/my-lib/.eslintrc.json').overrides + ).toContainEqual({ + files: ['*.json'], + parser: 'jsonc-eslint-parser', + rules: { + '@nx/dependency-checks': [ + 'error', + { + ignoredFiles: ['{projectRoot}/vite.config.{js,ts,mjs,mts}'], + }, + ], + }, + }); }); it.each` @@ -1159,6 +1173,66 @@ describe('lib', () => { ); }); + describe('--bundler=esbuild', () => { + it('should add build with esbuild', async () => { + await libraryGenerator(tree, { + ...defaultOptions, + name: 'myLib', + bundler: 'esbuild', + unitTestRunner: 'none', + }); + + const project = readProjectConfiguration(tree, 'my-lib'); + expect(project.targets.build).toMatchObject({ + executor: '@nx/esbuild:esbuild', + }); + expect( + readJson(tree, 'libs/my-lib/.eslintrc.json').overrides + ).toContainEqual({ + files: ['*.json'], + parser: 'jsonc-eslint-parser', + rules: { + '@nx/dependency-checks': [ + 'error', + { + ignoredFiles: ['{projectRoot}/esbuild.config.{js,ts,mjs,mts}'], + }, + ], + }, + }); + }); + }); + + describe('--bundler=rollup', () => { + it('should add build with rollup', async () => { + await libraryGenerator(tree, { + ...defaultOptions, + name: 'myLib', + bundler: 'rollup', + unitTestRunner: 'none', + }); + + const project = readProjectConfiguration(tree, 'my-lib'); + expect(project.targets.build).toMatchObject({ + executor: '@nx/rollup:rollup', + }); + expect( + readJson(tree, 'libs/my-lib/.eslintrc.json').overrides + ).toContainEqual({ + files: ['*.json'], + parser: 'jsonc-eslint-parser', + rules: { + '@nx/dependency-checks': [ + 'error', + { + ignoredFiles: ['{projectRoot}/rollup.config.{js,ts,mjs,mts}'], + }, + ], + }, + }); + }); + }); + describe('--minimal', () => { it('should generate a README.md when minimal is set to false', async () => { await libraryGenerator(tree, { diff --git a/packages/js/src/generators/library/library.ts b/packages/js/src/generators/library/library.ts index 810e7dc4a7344..652ae846d5323 100644 --- a/packages/js/src/generators/library/library.ts +++ b/packages/js/src/generators/library/library.ts @@ -10,6 +10,7 @@ import { names, offsetFromRoot, ProjectConfiguration, + readProjectConfiguration, runTasksInSerial, toJS, Tree, @@ -236,12 +237,14 @@ export type AddLintOptions = Pick< | 'js' | 'setParserOptionsProject' | 'rootProject' + | 'bundler' >; export async function addLint( tree: Tree, options: AddLintOptions ): Promise { const { lintProjectGenerator } = ensurePackage('@nx/linter', nxVersion); + const projectConfiguration = readProjectConfiguration(tree, options.name); const task = lintProjectGenerator(tree, { project: options.name, linter: options.linter, @@ -256,15 +259,17 @@ export async function addLint( setParserOptionsProject: options.setParserOptionsProject, rootProject: options.rootProject, }); + const { + addOverrideToLintConfig, + lintConfigHasOverride, + isEslintConfigSupported, + updateOverrideInLintConfig, + // nx-ignore-next-line + } = require('@nx/linter/src/generators/utils/eslint-file'); + // Also update the root ESLint config. The lintProjectGenerator will not generate it for root projects. // But we need to set the package.json checks. if (options.rootProject) { - const { - addOverrideToLintConfig, - isEslintConfigSupported, - // nx-ignore-next-line - } = require('@nx/linter/src/generators/utils/eslint-file'); - if (isEslintConfigSupported(tree)) { addOverrideToLintConfig(tree, '', { files: ['*.json'], @@ -275,6 +280,56 @@ export async function addLint( }); } } + + // If project lints package.json with @nx/dependency-checks, then add ignore files for + // build configuration files such as vite.config.ts. These config files need to be + // ignored, otherwise we will errors on missing dependencies that are for dev only. + if ( + lintConfigHasOverride( + tree, + projectConfiguration.root, + (o) => + Array.isArray(o.files) + ? o.files.some((f) => f.match(/\.json$/)) + : !!o.files?.match(/\.json$/), + true + ) + ) { + updateOverrideInLintConfig( + tree, + projectConfiguration.root, + (o) => o.rules?.['@nx/dependency-checks'], + (o) => { + const value = o.rules['@nx/dependency-checks']; + let ruleSeverity: string; + let ruleOptions: any; + if (Array.isArray(value)) { + ruleSeverity = value[0]; + ruleOptions = value[1]; + } else { + ruleSeverity = value; + ruleOptions = {}; + } + if (options.bundler === 'vite' || options.unitTestRunner === 'vitest') { + ruleOptions.ignoredFiles = [ + '{projectRoot}/vite.config.{js,ts,mjs,mts}', + ]; + o.rules['@nx/dependency-checks'] = [ruleSeverity, ruleOptions]; + } else if (options.bundler === 'rollup') { + ruleOptions.ignoredFiles = [ + '{projectRoot}/rollup.config.{js,ts,mjs,mts}', + ]; + o.rules['@nx/dependency-checks'] = [ruleSeverity, ruleOptions]; + } else if (options.bundler === 'esbuild') { + ruleOptions.ignoredFiles = [ + '{projectRoot}/esbuild.config.{js,ts,mjs,mts}', + ]; + o.rules['@nx/dependency-checks'] = [ruleSeverity, ruleOptions]; + } + return o; + } + ); + } return task; } diff --git a/packages/js/src/utils/find-npm-dependencies.spec.ts b/packages/js/src/utils/find-npm-dependencies.spec.ts index d5f0f4cd16cff..9eebff3ce114b 100644 --- a/packages/js/src/utils/find-npm-dependencies.spec.ts +++ b/packages/js/src/utils/find-npm-dependencies.spec.ts @@ -397,4 +397,57 @@ describe('findNpmDependencies', () => { '@acme/lib3': '*', }); }); + + it('should support ignoring extra file patterns in addition to task input', () => { + vol.fromJSON( + { + './nx.json': JSON.stringify(nxJson), + }, + '/root' + ); + const lib = { + name: 'my-lib', + type: 'lib' as const, + data: { + root: 'libs/my-lib', + targets: { build: {} }, + }, + }; + const projectGraph = { + nodes: { + 'my-lib': lib, + }, + externalNodes: { + 'npm:foo': { + name: 'npm:foo' as const, + type: 'npm' as const, + data: { + packageName: 'foo', + version: '1.0.0', + }, + }, + }, + dependencies: {}, + }; + const projectFileMap = { + 'my-lib': [ + { + file: 'libs/my-lib/vite.config.ts', + hash: '123', + deps: ['npm:foo'], + }, + ], + }; + + const results = findNpmDependencies( + '/root', + lib, + projectGraph, + projectFileMap, + 'build', + { ignoredFiles: ['{projectRoot}/vite.config.ts'] } + ); + + expect(results).toEqual({}); + }); }); diff --git a/packages/js/src/utils/find-npm-dependencies.ts b/packages/js/src/utils/find-npm-dependencies.ts index 5fe5ee78294c5..b913138e7f32e 100644 --- a/packages/js/src/utils/find-npm-dependencies.ts +++ b/packages/js/src/utils/find-npm-dependencies.ts @@ -27,6 +27,7 @@ export function findNpmDependencies( buildTarget: string, options: { includeTransitiveDependencies?: boolean; + ignoredFiles?: string[]; } = {} ): Record { let seen: null | Set = null; @@ -41,6 +42,7 @@ export function findNpmDependencies( collectedDeps: Record ): void { if (seen?.has(currentProject.name)) return; + seen?.add(currentProject.name); collectDependenciesFromFileMap( workspaceRoot, @@ -48,6 +50,7 @@ export function findNpmDependencies( projectGraph, projectFileMap, buildTarget, + options.ignoredFiles, collectedDeps ); @@ -82,19 +85,22 @@ function collectDependenciesFromFileMap( projectGraph: ProjectGraph, projectFileMap: ProjectFileMap, buildTarget: string, + ignoredFiles: string[], npmDeps: Record ): void { const rawFiles = projectFileMap[sourceProject.name]; if (!rawFiles) return; - // Cannot read inputs if the target does not exist on the project. - if (!sourceProject.data.targets[buildTarget]) return; - - const inputs = getTargetInputs( - readNxJson(), - sourceProject, - buildTarget - ).selfInputs; + // If build target does not exist in project, use all files as input. + // This is needed for transitive dependencies for apps -- where libs may not be buildable. + const inputs = sourceProject.data.targets[buildTarget] + ? getTargetInputs(readNxJson(), sourceProject, buildTarget).selfInputs + : ['{projectRoot}/**/*']; + if (ignoredFiles) { + for (const pattern of ignoredFiles) { + inputs.push(`!${pattern}`); + } + } const files = filterUsingGlobPatterns( sourceProject.data.root, projectFileMap[sourceProject.name] || [], @@ -128,7 +134,12 @@ function collectDependenciesFromFileMap( npmDeps[cached.name] = cached.version; } else { const packageJson = readPackageJson(workspaceDep, workspaceRoot); - if (packageJson) { + if ( + // Check that this is a buildable project, otherwise it cannot be a dependency in package.json. + workspaceDep.data.targets[buildTarget] && + // Make sure package.json exists and has a valid name. + packageJson?.name + ) { // This is a workspace lib so we can't reliably read in a specific version since it depends on how the workspace is set up. // ASSUMPTION: Most users will use '*' for workspace lib versions. Otherwise, they can manually update it. npmDeps[packageJson.name] = '*';