Skip to content

Commit

Permalink
fix(core): target defaults should represent nx.json in source info
Browse files Browse the repository at this point in the history 
Also fixes case where target defaults was incorrectly creating targets for a script
  • Loading branch information
@AgentEnder
AgentEnder committed Feb 29, 2024
1 parent f046c52 commit 58d866d
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 12 deletions.
98 changes: 94 additions & 4 deletions packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
Expand All @@ -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": {
Expand Down Expand Up @@ -110,6 +112,7 @@ describe('target-defaults plugin', () => {
})
).toMatchInlineSnapshot(`
{
"NX_OVERRIDE_SOURCE_FILE": "nx.json",
"projects": {
".": {
"targets": {
Expand Down Expand Up @@ -156,6 +159,7 @@ describe('target-defaults plugin', () => {
})
).toMatchInlineSnapshot(`
{
"NX_OVERRIDE_SOURCE_FILE": "nx.json",
"projects": {
".": {
"targets": {
Expand Down Expand Up @@ -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,
},
},
},
Expand All @@ -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(
Expand Down Expand Up @@ -279,6 +367,7 @@ describe('target-defaults plugin', () => {
expect(createNodesFn('project.json', undefined, context))
.toMatchInlineSnapshot(`
{
"NX_OVERRIDE_SOURCE_FILE": "nx.json",
"projects": {
".": {
"targets": {
Expand All @@ -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,
},
},
},
Expand Down Expand Up @@ -343,6 +432,7 @@ describe('target-defaults plugin', () => {
expect(createNodesFn('project.json', undefined, context))
.toMatchInlineSnapshot(`
{
"NX_OVERRIDE_SOURCE_FILE": "nx.json",
"projects": {
".": {
"targets": {
Expand All @@ -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,
},
},
},
Expand Down
17 changes: 11 additions & 6 deletions packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ 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.
*/
export const ONLY_MODIFIES_EXISTING_TARGET = Symbol(
'ONLY_MODIFIES_EXISTING_TARGET'
);
export const ONLY_MODIFIES_EXISTING_TARGET = 'NX_ONLY_MODIFIES_EXISTING_TARGET';

/**
* This symbol 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.
*/
export const OVERRIDE_SOURCE_FILE = 'NX_OVERRIDE_SOURCE_FILE';

export const TargetDefaultsPlugin: NxPluginV2 = {
name: 'nx/core/target-defaults',
Expand Down Expand Up @@ -111,6 +115,7 @@ export const TargetDefaultsPlugin: NxPluginV2 = {
targets: modifiedTargets,
},
},
[OVERRIDE_SOURCE_FILE]: 'nx.json',
};
},
],
Expand Down Expand Up @@ -186,7 +191,7 @@ export function getTargetInfo(
return {
executor: 'nx:run-commands',
options: {
command: projectJsonTarget.options?.command,
command: targetOptions?.command,
},
};
} else if (targetOptions?.commands) {
Expand Down Expand Up @@ -227,7 +232,7 @@ function getTargetExecutor(
const packageJsonTargetConfiguration = packageJsonTargets?.[target];

if (!projectJsonTargetConfiguration && packageJsonTargetConfiguration) {
return packageJsonTargetConfiguration?.executor ?? 'nx:run-script';
return packageJsonTargetConfiguration?.executor;
}

if (projectJsonTargetConfiguration?.executor) {
Expand Down
14 changes: 12 additions & 2 deletions packages/nx/src/project-graph/utils/project-configuration-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -254,6 +257,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,
Expand All @@ -264,7 +274,7 @@ export function buildProjectsConfigurationsFromProjectPathsAndPlugins(
projectRootMap,
project,
configurationSourceMaps,
[file, pluginName]
sourceInfo
);
} catch (e) {
throw new CreateNodesError(
Expand Down
61 changes: 61 additions & 0 deletions packages/nx/src/utils/package-json.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 58d866d

Please sign in to comment.