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 committed Feb 29, 2024
1 parent f046c52 commit f6ceb05
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,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
15 changes: 10 additions & 5 deletions packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts
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 @@ -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
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
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 f6ceb05

Please sign in to comment.