Skip to content

Commit

Permalink
fix(core): keep all explicit dependencies in the graph (#16576)
Browse files Browse the repository at this point in the history
  • Loading branch information
meeroslav authored May 9, 2023
1 parent 50ad516 commit ae5cdcb
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 51 deletions.
2 changes: 1 addition & 1 deletion e2e/linter/src/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ describe('Linter', () => {
import '@${projScope}/${invalidtaglib}';
import '@${projScope}/${validtaglib}';
const s = {loadChildren: '@${projScope}/${lazylib}'};
const s = {loadChildren: '@secondScope/${lazylib}'};
`
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ function findAllNpmDeps(
seen,
dependencyPatterns
);
} else {
throw new Error(`Could not find ${dep} in the project graph.`);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,28 @@ export function buildExplicitTypeScriptDependencies(
importExpr,
f.file
);
let targetProjectName;
if (target) {
if (!isRoot(source) && isRoot(target)) {
// TODO: These edges technically should be allowed but we need to figure out how to separate config files out from root
return;
}

res.push({
sourceProjectName: source,
targetProjectName: target,
sourceProjectFile: f.file,
type,
});
targetProjectName = target;
} else {
// treat all unknowns as npm packages, they can be eiher
// - mistyped local import, which has to be fixed manually
// - node internals, which should still be tracked as a dependency
// - npm packages, which are not yet installed but should be tracked
targetProjectName = `npm:${importExpr}`;
}

res.push({
sourceProjectName: source,
targetProjectName,
sourceProjectFile: f.file,
type,
});
}
);
});
Expand Down
9 changes: 2 additions & 7 deletions packages/nx/src/project-graph/project-graph-builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ describe('ProjectGraphBuilder', () => {
expect(() =>
builder.addImplicitDependency('source', 'invalid-target')
).toThrowError();
// this should not break, but should not exist in resulting dependencies either
builder.addStaticDependency('source', 'invalid-target', 'source/index.ts');

// ignore the self deps
builder.addDynamicDependency('source', 'source', 'source/index.ts');
Expand Down Expand Up @@ -91,13 +93,6 @@ describe('ProjectGraphBuilder', () => {
'target'
)
).toThrowError();
expect(() =>
builder.addExplicitDependency(
'source',
'source/index.ts',
'invalid-target'
)
).toThrowError();
expect(() =>
builder.addExplicitDependency(
'source',
Expand Down
48 changes: 13 additions & 35 deletions packages/nx/src/project-graph/project-graph-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,41 +175,11 @@ export class ProjectGraphBuilder {
sourceProjectFile: string,
targetProjectName: string
): void {
if (sourceProjectName === targetProjectName) {
return;
}
const source = this.graph.nodes[sourceProjectName];
if (!source) {
throw new Error(`Source project does not exist: ${sourceProjectName}`);
}

if (
!this.graph.nodes[targetProjectName] &&
!this.graph.externalNodes[targetProjectName]
) {
throw new Error(`Target project does not exist: ${targetProjectName}`);
}

const fileData = source.data.files.find(
(f) => f.file === sourceProjectFile
this.addStaticDependency(
sourceProjectName,
targetProjectName,
sourceProjectFile
);
if (!fileData) {
throw new Error(
`Source project ${sourceProjectName} does not have a file: ${sourceProjectFile}`
);
}

if (!fileData.dependencies) {
fileData.dependencies = [];
}

if (!fileData.dependencies.find((t) => t.target === targetProjectName)) {
fileData.dependencies.push({
target: targetProjectName,
source: sourceProjectName,
type: DependencyType.static,
});
}
}

/**
Expand All @@ -229,6 +199,13 @@ export class ProjectGraphBuilder {

const fileDeps = this.calculateTargetDepsFromFiles(sourceProject);
for (const [targetProject, types] of fileDeps.entries()) {
// only add known nodes
if (
!this.graph.nodes[targetProject] &&
!this.graph.externalNodes[targetProject]
) {
continue;
}
for (const type of types.values()) {
if (
!alreadySetTargetProjects.has(targetProject) ||
Expand Down Expand Up @@ -268,7 +245,8 @@ export class ProjectGraphBuilder {
}
if (
!this.graph.nodes[targetProjectName] &&
!this.graph.externalNodes[targetProjectName]
!this.graph.externalNodes[targetProjectName] &&
!sourceProjectFile
) {
throw new Error(`Target project does not exist: ${targetProjectName}`);
}
Expand Down

1 comment on commit ae5cdcb

@vercel
Copy link

@vercel vercel bot commented on ae5cdcb May 9, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

nx-dev – ./

nx-dev-nrwl.vercel.app
nx-five.vercel.app
nx.dev
nx-dev-git-master-nrwl.vercel.app

Please sign in to comment.