From a6613547099010b8c3f4575191e3a93e2f9208ef Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Fri, 1 Mar 2024 10:04:33 -0500 Subject: [PATCH] fix(core): target defaults should represent nx.json in source info (#22080) --- .../target-defaults-plugin.spec.ts | 98 ++++++++++++++++++- .../target-defaults/target-defaults-plugin.ts | 23 +++-- .../utils/project-configuration-utils.ts | 14 ++- packages/nx/src/utils/package-json.spec.ts | 61 ++++++++++++ 4 files changed, 184 insertions(+), 12 deletions(-) diff --git a/packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts b/packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts index 1ec89af52f174..2c08278075c4f 100644 --- a/packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts +++ b/packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts @@ -50,14 +50,15 @@ describe('target-defaults plugin', () => { expect(createNodesFn('project.json', undefined, context)) .toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { "build": { + "NX_ONLY_MODIFIES_EXISTING_TARGET": true, "dependsOn": [ "^build", ], - Symbol(ONLY_MODIFIES_EXISTING_TARGET): true, }, }, }, @@ -67,6 +68,7 @@ describe('target-defaults plugin', () => { expect(createNodesFn('packages/lib-a/project.json', undefined, context)) .toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { "packages/lib-a": { "targets": { @@ -110,6 +112,7 @@ describe('target-defaults plugin', () => { }) ).toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { @@ -156,6 +159,7 @@ describe('target-defaults plugin', () => { }) ).toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { @@ -229,12 +233,13 @@ describe('target-defaults plugin', () => { }) ).toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { "test": { + "NX_ONLY_MODIFIES_EXISTING_TARGET": true, "command": "jest", - Symbol(ONLY_MODIFIES_EXISTING_TARGET): true, }, }, }, @@ -243,6 +248,89 @@ describe('target-defaults plugin', () => { `); }); + it('should not register target if target default and package.json target if package.json target is not included script', async () => { + memfs.vol.fromJSON( + { + 'package.json': JSON.stringify({ + name: 'lib-a', + scripts: { + test: 'nx affected:test', + }, + nx: { + includedScripts: [], + targets: { + test: { + outputs: ['coverage'], + }, + }, + }, + }), + }, + '/root' + ); + + const result = await createNodesFn('package.json', undefined, { + nxJsonConfiguration: { + targetDefaults: { + test: { + cache: true, + }, + }, + }, + workspaceRoot: '/root', + }); + + const { targets } = result.projects['.']; + + // Info from target defaults will be merged + expect(targets.test.cache).toBeTruthy(); + // Info from package.json will not be merged at this time - it will be merged when processing package json plugin + expect(targets.test.outputs).not.toBeDefined(); + }); + + it('should register target if target default and package.json target if package.json target is not included script but has executor', async () => { + memfs.vol.fromJSON( + { + 'package.json': JSON.stringify({ + name: 'lib-a', + scripts: { + test: 'nx affected:test', + }, + nx: { + includedScripts: [], + targets: { + test: { + executor: 'nx:run-commands', + options: { + command: 'echo hi', + }, + }, + }, + }, + }), + }, + '/root' + ); + + const result = await createNodesFn('package.json', undefined, { + nxJsonConfiguration: { + targetDefaults: { + test: { + cache: true, + }, + }, + }, + workspaceRoot: '/root', + }); + + const { targets } = result.projects['.']; + + // Info from target defaults will be merged + expect(targets.test.cache).toBeTruthy(); + // Info from package.json will be merged so that the target default is compatible + expect(targets.test.executor).toEqual('nx:run-commands'); + }); + describe('executor key', () => { it('should support multiple targets with the same executor', () => { memfs.vol.fromJSON( @@ -279,6 +367,7 @@ describe('target-defaults plugin', () => { expect(createNodesFn('project.json', undefined, context)) .toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { @@ -295,10 +384,10 @@ describe('target-defaults plugin', () => { }, }, "nx:run-commands": { + "NX_ONLY_MODIFIES_EXISTING_TARGET": true, "options": { "cwd": "{projectRoot}", }, - Symbol(ONLY_MODIFIES_EXISTING_TARGET): true, }, }, }, @@ -343,6 +432,7 @@ describe('target-defaults plugin', () => { expect(createNodesFn('project.json', undefined, context)) .toMatchInlineSnapshot(` { + "NX_OVERRIDE_SOURCE_FILE": "nx.json", "projects": { ".": { "targets": { @@ -359,10 +449,10 @@ describe('target-defaults plugin', () => { }, }, "nx:run-commands": { + "NX_ONLY_MODIFIES_EXISTING_TARGET": true, "options": { "cwd": "{projectRoot}", }, - Symbol(ONLY_MODIFIES_EXISTING_TARGET): true, }, }, }, diff --git a/packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts b/packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts index 5266bbceee68c..f31c5ec1b8a32 100644 --- a/packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts +++ b/packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts @@ -16,13 +16,23 @@ import { import { getGlobPatternsFromPackageManagerWorkspaces } from '../package-json-workspaces'; /** - * This symbol marks that a target provides information which should modify a target already registered + * This marks that a target provides information which should modify a target already registered * on the project via other plugins. If the target has not already been registered, and this symbol is true, * the information provided by it will be discarded. + * + * NOTE: This cannot be a symbol, as they are not serialized in JSON the communication + * between the plugin-worker and the main process. */ -export const ONLY_MODIFIES_EXISTING_TARGET = Symbol( - 'ONLY_MODIFIES_EXISTING_TARGET' -); +export const ONLY_MODIFIES_EXISTING_TARGET = 'NX_ONLY_MODIFIES_EXISTING_TARGET'; + +/** + * This is used to override the source file for the target defaults plugin. + * This allows the plugin to use the project files as the context, but point to nx.json as the source file. + * + * NOTE: This cannot be a symbol, as they are not serialized in JSON the communication + * between the plugin-worker and the main process. + */ +export const OVERRIDE_SOURCE_FILE = 'NX_OVERRIDE_SOURCE_FILE'; export const TargetDefaultsPlugin: NxPluginV2 = { name: 'nx/core/target-defaults', @@ -111,6 +121,7 @@ export const TargetDefaultsPlugin: NxPluginV2 = { targets: modifiedTargets, }, }, + [OVERRIDE_SOURCE_FILE]: 'nx.json', }; }, ], @@ -186,7 +197,7 @@ export function getTargetInfo( return { executor: 'nx:run-commands', options: { - command: projectJsonTarget.options?.command, + command: targetOptions?.command, }, }; } else if (targetOptions?.commands) { @@ -227,7 +238,7 @@ function getTargetExecutor( const packageJsonTargetConfiguration = packageJsonTargets?.[target]; if (!projectJsonTargetConfiguration && packageJsonTargetConfiguration) { - return packageJsonTargetConfiguration?.executor ?? 'nx:run-script'; + return packageJsonTargetConfiguration?.executor; } if (projectJsonTargetConfiguration?.executor) { diff --git a/packages/nx/src/project-graph/utils/project-configuration-utils.ts b/packages/nx/src/project-graph/utils/project-configuration-utils.ts index 2de13e24e296d..c609b7d9b7c0c 100644 --- a/packages/nx/src/project-graph/utils/project-configuration-utils.ts +++ b/packages/nx/src/project-graph/utils/project-configuration-utils.ts @@ -7,7 +7,10 @@ import { import { NX_PREFIX } from '../../utils/logger'; import { readJsonFile } from '../../utils/fileutils'; import { workspaceRoot } from '../../utils/workspace-root'; -import { ONLY_MODIFIES_EXISTING_TARGET } from '../../plugins/target-defaults/target-defaults-plugin'; +import { + ONLY_MODIFIES_EXISTING_TARGET, + OVERRIDE_SOURCE_FILE, +} from '../../plugins/target-defaults/target-defaults-plugin'; import { minimatch } from 'minimatch'; import { join } from 'path'; @@ -252,6 +255,13 @@ export function buildProjectsConfigurationsFromProjectPathsAndPlugins( file, pluginName, } = result; + + const sourceInfo: SourceInformation = [file, pluginName]; + + if (result[OVERRIDE_SOURCE_FILE]) { + sourceInfo[0] = result[OVERRIDE_SOURCE_FILE]; + } + for (const node in projectNodes) { const project = { root: node, @@ -262,7 +272,7 @@ export function buildProjectsConfigurationsFromProjectPathsAndPlugins( projectRootMap, project, configurationSourceMaps, - [file, pluginName] + sourceInfo ); } catch (e) { throw new CreateNodesError( diff --git a/packages/nx/src/utils/package-json.spec.ts b/packages/nx/src/utils/package-json.spec.ts index 3c02952ed75f1..7503762451d5d 100644 --- a/packages/nx/src/utils/package-json.spec.ts +++ b/packages/nx/src/utils/package-json.spec.ts @@ -211,6 +211,67 @@ describe('readTargetsFromPackageJson', () => { } `); }); + + it('should support partial target info without including script', () => { + const result = readTargetsFromPackageJson({ + name: 'my-remix-app-8cce', + version: '', + scripts: { + build: 'run-s build:*', + 'build:icons': 'tsx ./other/build-icons.ts', + 'build:remix': 'remix build --sourcemap', + 'build:server': 'tsx ./other/build-server.ts', + predev: 'npm run build:icons --silent', + dev: 'remix dev -c "node ./server/dev-server.js" --manual', + 'prisma:studio': 'prisma studio', + format: 'prettier --write .', + lint: 'eslint .', + setup: + 'npm run build && prisma generate && prisma migrate deploy && prisma db seed && playwright install', + start: 'cross-env NODE_ENV=production node .', + 'start:mocks': 'cross-env NODE_ENV=production MOCKS=true tsx .', + test: 'vitest', + coverage: 'nx test --coverage', + 'test:e2e': 'npm run test:e2e:dev --silent', + 'test:e2e:dev': 'playwright test --ui', + 'pretest:e2e:run': 'npm run build', + 'test:e2e:run': 'cross-env CI=true playwright test', + 'test:e2e:install': 'npx playwright install --with-deps chromium', + typecheck: 'tsc', + validate: 'run-p "test -- --run" lint typecheck test:e2e:run', + }, + nx: { + targets: { + 'build:icons': { + outputs: ['{projectRoot}/app/components/ui/icons'], + }, + 'build:remix': { + outputs: ['{projectRoot}/build'], + }, + 'build:server': { + outputs: ['{projectRoot}/server-build'], + }, + test: { + outputs: ['{projectRoot}/test-results'], + }, + 'test:e2e': { + outputs: ['{projectRoot}/playwright-report'], + }, + 'test:e2e:run': { + outputs: ['{projectRoot}/playwright-report'], + }, + }, + includedScripts: [], + }, + }); + expect(result.test).toMatchInlineSnapshot(` + { + "outputs": [ + "{projectRoot}/test-results", + ], + } + `); + }); }); const rootPackageJson: PackageJson = readJsonFile(