Skip to content

Commit

Permalink
fix(core): project graph should reuse methods to combine target defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
AgentEnder committed Dec 27, 2022
1 parent 7d386d1 commit b884269
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 101 deletions.
4 changes: 2 additions & 2 deletions docs/shared/reference/nx-json.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ In this case Nx will use the right `production` input for each project.

Target defaults provide ways to set common options for a particular target in your workspace. When building your project's configuration, we merge it with up to 1 default from this map. For a given target, we look at its name and its executor. We then check target defaults for any of the following combinations:

- `` `${targetName}|${executor}` ``
- `` `*|${executor}` ``
- `` `${targetName}#${executor}` ``
- `` `${executor}` ``
- `` `${targetName}` ``

Whichever of these we find first, we use as the base for that target's configuration. Some common scenarios for this follow.
Expand Down
14 changes: 9 additions & 5 deletions e2e/nx-run/src/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,14 @@ describe('Nx Running Tests', () => {
JSON.stringify({
name: lib,
targets: {
target: {},
[target]: {},
},
})
);

expect(runCLI(`${target} ${lib}`)).toContain(`Hello from ${target}`);
expect(runCLI(`${target} ${lib} --verbose`)).toContain(
`Hello from ${target}`
);
});

it('should be able to pull options from targetDefaults based on executor', () => {
Expand All @@ -289,7 +291,7 @@ describe('Nx Running Tests', () => {

updateJson('nx.json', (nxJson) => {
nxJson.targetDefaults ??= {};
nxJson.targetDefaults[`*|nx:run-commands}`] = {
nxJson.targetDefaults[`nx:run-commands`] = {
options: {
command: `echo Hello from ${target}`,
},
Expand All @@ -302,14 +304,16 @@ describe('Nx Running Tests', () => {
JSON.stringify({
name: lib,
targets: {
target: {
[target]: {
executor: 'nx:run-commands',
},
},
})
);

expect(runCLI(`${target} ${lib}`)).toContain(`Hello from ${target}`);
expect(runCLI(`${target} ${lib} --verbose`)).toContain(
`Hello from ${target}`
);
});
});

Expand Down
44 changes: 20 additions & 24 deletions packages/nx/schemas/nx-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,26 @@
"type": "object",
"description": "Target defaults",
"properties": {
"executor": {
"description": "The function that Nx will invoke when you run this target",
"type": "string"
},
"options": {
"type": "object"
},
"outputs": {
"type": "array",
"items": {
"type": "string"
}
},
"configurations": {
"type": "object",
"description": "provides extra sets of values that will be merged into the options map",
"additionalProperties": {
"type": "object"
}
},
"inputs": {
"$ref": "#/definitions/inputs"
},
Expand Down Expand Up @@ -243,32 +263,8 @@
}
]
}
},
"outputs": {
"type": "array",
"items": {
"type": "string"
}
},
"executor": {
"description": "The function that Nx will invoke when you run this target",
"type": "string"
},
"options": {
"type": "object"
},
"configurations": {
"type": "object",
"description": "provides extra sets of values that will be merged into the options map",
"additionalProperties": {
"type": "object"
}
}
},
"dependentRequired": {
"configurations": ["executor"],
"options": ["executor"]
},
"additionalProperties": false
}
}
Expand Down
97 changes: 62 additions & 35 deletions packages/nx/src/config/workspaces.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,52 +145,55 @@ describe('Workspaces', () => {
});

describe('target defaults', () => {
const nxJson = {
targetDefaults: {
'build|nx:run-commands': {
options: {
key: 't:e',
},
const targetDefaults = {
'build#nx:run-commands': {
options: {
key: 't:e',
},
'*|nx:run-commands': {
options: {
key: '*:e',
},
},
'nx:run-commands': {
options: {
key: '*:e',
},
build: {
options: {
key: 't',
},
},
build: {
options: {
key: 't',
},
},
};

it('should prefer target|executor key', () => {
it('should prefer target#executor key', () => {
expect(
readTargetDefaultsForTarget('build', nxJson, 'run-commands').options[
'key'
]
readTargetDefaultsForTarget('build', targetDefaults, 'nx:run-commands')
.options['key']
).toEqual('t:e');
});

it('should prefer *|executor key', () => {
it('should prefer executor key', () => {
expect(
readTargetDefaultsForTarget('other-target', nxJson, 'run-commands')
.options['key']
readTargetDefaultsForTarget(
'other-target',
targetDefaults,
'nx:run-commands'
).options['key']
).toEqual('*:e');
});

it('should fallback to target key', () => {
expect(
readTargetDefaultsForTarget('build', nxJson, 'other-executor').options[
'key'
]
readTargetDefaultsForTarget('build', targetDefaults, 'other-executor')
.options['key']
).toEqual('t');
});

it('should return undefined if not found', () => {
expect(
readTargetDefaultsForTarget('other-target', nxJson, 'other-executor')
readTargetDefaultsForTarget(
'other-target',
targetDefaults,
'other-executor'
)
).toBeNull();
});

Expand All @@ -200,11 +203,17 @@ describe('Workspaces', () => {
expect(
mergeTargetConfigurations(
{
executor: 'target',
[property]: {
a: {},
root: '.',
targets: {
build: {
executor: 'target',
[property]: {
a: {},
},
},
},
},
'build',
{
executor: 'target',
[property]: {
Expand All @@ -223,11 +232,17 @@ describe('Workspaces', () => {
expect(
mergeTargetConfigurations(
{
executor: 'target',
[property]: {
a: {},
root: '',
targets: {
build: {
executor: 'target',
[property]: {
a: {},
},
},
},
},
'build',
{
[property]: {
b: {},
Expand All @@ -244,10 +259,16 @@ describe('Workspaces', () => {
expect(
mergeTargetConfigurations(
{
[property]: {
a: {},
root: '',
targets: {
build: {
[property]: {
a: {},
},
},
},
},
'build',
{
executor: 'target',
[property]: {
Expand All @@ -265,10 +286,16 @@ describe('Workspaces', () => {
expect(
mergeTargetConfigurations(
{
[property]: {
a: {},
root: '',
targets: {
build: {
[property]: {
a: {},
},
},
},
},
'build',
{
executor: 'target',
[property]: {
Expand Down
63 changes: 49 additions & 14 deletions packages/nx/src/config/workspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ export class Workspaces {
const projectTargetDefinition = proj.targets[targetName];
const defaults = readTargetDefaultsForTarget(
targetName,
nxJson,
nxJson.targetDefaults,
projectTargetDefinition.executor
);

if (defaults) {
const defaults = nxJson.targetDefaults[targetName];
proj.targets[targetName] = mergeTargetConfigurations(
projectTargetDefinition,
proj,
targetName,
defaults
);
}
Expand Down Expand Up @@ -921,9 +921,12 @@ export function buildWorkspaceConfigurationFromGlobs(
}

export function mergeTargetConfigurations(
targetConfiguration: Partial<TargetConfiguration>,
projectConfiguration: ProjectConfiguration,
target: string,
targetDefaults: TargetDefaults[string]
): TargetConfiguration {
const { targets, root } = projectConfiguration;
const targetConfiguration = targets?.[target] ?? {};
const {
configurations: defaultConfigurations,
options: defaultOptions,
Expand All @@ -934,7 +937,12 @@ export function mergeTargetConfigurations(
...targetConfiguration,
};

if (!targetDefaults.executor && !targetConfiguration.executor) {
if (
!targetDefaults.executor &&
!targetConfiguration.executor &&
!targetDefaults.command &&
!targetConfiguration.command
) {
throw new Error('Unable to determine executor for target');
}

Expand All @@ -946,7 +954,7 @@ export function mergeTargetConfigurations(
targetDefaults.executor === targetConfiguration.executor
) {
result.options = {
...defaultOptions,
...resolvePathTokensInOptions(defaultOptions, root, target),
...targetConfiguration.options,
};
result.configurations = {
Expand All @@ -957,28 +965,55 @@ export function mergeTargetConfigurations(
return result as TargetConfiguration;
}

function resolvePathTokensInOptions<T extends Object>(
object: T,
projectRoot: string,
key: string
): T {
for (let [opt, value] of Object.entries(object ?? {})) {
if (typeof value === 'string') {
if (value.startsWith('{workspaceRoot}/')) {
value = value.replace(/^\{workspaceRoot\}\//, '');
}
if (value.includes('{workspaceRoot}')) {
output.error({
title: `${NX_PREFIX}: {workspaceRoot} is only valid at the beginning of an option`,
bodyLines: [`Found {workspaceRoot} in property: ${key}`],
});
throw new Error(
'{workspaceRoot} is only valid at the beginning of an option.'
);
}
object[opt] = value.replace('{projectRoot}', projectRoot);
} else if (typeof value === 'object' && value) {
resolvePathTokensInOptions(value, projectRoot, [key, opt].join('.'));
}
}
return object;
}

export function readTargetDefaultsForTarget(
targetName: string,
nxJson: NxJsonConfiguration<'*' | string[]>,
targetDefaults: TargetDefaults,
executor?: string
): TargetDefaults[string] {
if (executor) {
// If an executor is defined in project.json, defaults should be read
// from the most specific key that matches that executor.
// e.g. If executor === run-commands, and the target is named build:
// Use build|run-commands if it is present
// If not, use *|run-commands if it is present
// Use build#nx:run-commands if it is present
// If not, use nx:run-commands if it is present
// If not, use build if it is present.
const separator = '|';
const separator = '#';
const key = [
`${targetName}${separator}${executor}`,
`*${separator}${executor}`,
executor,
targetName,
].find((x) => nxJson.targetDefaults[x]);
return key ? nxJson.targetDefaults[key] : null;
].find((x) => targetDefaults?.[x]);
return key ? targetDefaults?.[key] : null;
} else {
// If the executor is not defined, the only key we have is the target name.
return nxJson.targetDefaults[targetName];
return targetDefaults?.[targetName];
}
}

Expand Down
Loading

0 comments on commit b884269

Please sign in to comment.