Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): target defaults should represent nx.json in source info #22080

Merged
merged 1 commit into from Mar 1, 2024
Merged

fix(core): target defaults should represent nx.json in source info #22080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
98 changes: 94 additions & 4 deletions packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts
View file
Open in desktop
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
23 changes: 17 additions & 6 deletions packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts
View file
Open in desktop
Expand Up @@ -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',
Expand Down Expand Up @@ -111,6 +121,7 @@ export const TargetDefaultsPlugin: NxPluginV2 = {
targets: modifiedTargets,
},
},
[OVERRIDE_SOURCE_FILE]: 'nx.json',
};
},
],
Expand Down Expand Up @@ -186,7 +197,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 +238,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
Open in desktop
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
Open in desktop
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