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

fix(core): fix handling of simple dependsOn target string #16932

Merged
merged 1 commit into from
May 11, 2023
Merged

fix(core): fix handling of simple dependsOn target string #16932

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/nx/src/tasks-runner/utils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,25 @@ describe('utils', () => {
});
});

it('should assume target of self if simple target also matches project name', () => {
const result = expandDependencyConfigSyntaxSugar('build', {
dependencies: {},
nodes: {
build: {
name: 'build',
type: 'lib',
data: {
root: 'libs/build',
files: [],
},
},
},
});
expect(result).toEqual({
target: 'build',
});
});

it('should expand syntax for simple target names targetting dependencies', () => {
const result = expandDependencyConfigSyntaxSugar('^build', {
dependencies: {},
Expand Down
7 changes: 7 additions & 0 deletions packages/nx/src/tasks-runner/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ export function expandDependencyConfigSyntaxSugar(

// Support for both `project:target` and `target:with:colons` syntax
const [maybeProject, ...segments] = splitByColons(targetString);

// if no additional segments are provided, then the string references
// a target of the same project
if (!segments.length) {
return { target: maybeProject };
}
Comment on lines +75 to +77
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!segments.length) {
return { target: maybeProject };
}
if (segments.length == 1) {
return { target: maybeProject };
}

This condition as written would never be hit, since there is always at least 1 segment (that just happens to be the target name most of the time).

It's OK IMHO to assume it's a target name when only one segment is present, so updating the condition like above should suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AgentEnder I think this code is correct as written - a couple lines above there's the [maybeProject, ...segments] = splitByColons(targetString) line. Unless I'm misunderstanding how the split function works, it seems that segments does have the possibility to be empty 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also for what it's worth - I tested the change as I originally wrote it in my monorepo (via patch-package) and it did fix the issue I was experiencing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. That's what I get for checking on my phone!

Thanks for the contribution!


return {
// Only the first segment could be a project. If it is, the rest is a target.
// If its not, then the whole targetString was a target with colons in its name.
Expand Down