Skip to content

Commit

Permalink
fix(js): fix missing top-level dependencies in publishable libs (#17730)
Browse files Browse the repository at this point in the history
  • Loading branch information
daiscog authored Jan 29, 2024
1 parent 1ea09ad commit ec38a58
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 1 deletion.
95 changes: 95 additions & 0 deletions packages/js/src/utils/buildable-libs-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,101 @@ describe('calculateProjectDependencies', () => {
],
});
});

it('should include all top-level dependencies, even ones that are also transitive', async () => {
const graph: ProjectGraph = {
nodes: {
example: {
type: 'lib',
name: 'example',
data: {
root: '/root/example',
targets: {
build: {
executor: 'x',
},
},
},
},
example2: {
type: 'lib',
name: 'example2',
data: {
root: '/root/example2',
targets: {
build: {
executor: 'x',
},
},
},
},
},
externalNodes: {
'npm:formik': {
type: 'npm',
name: 'npm:formik',
data: {
packageName: 'formik',
version: '0.0.0',
},
},
'npm:foo': {
type: 'npm',
name: 'npm:foo',
data: {
packageName: 'foo',
version: '0.0.0',
},
},
},
dependencies: {
example: [
// when example2 dependency is listed first
{
source: 'example',
target: 'example2',
type: DependencyType.static,
},
{
source: 'example',
target: 'npm:formik',
type: DependencyType.static,
},
],
example2: [
// and example2 also depends on npm:formik
{
source: 'example2',
target: 'npm:formik',
type: DependencyType.static,
},
{
source: 'example2',
target: 'npm:foo',
type: DependencyType.static,
},
],
},
};

const results = calculateProjectDependencies(
graph,
'root',
'example',
'build',
undefined
);
expect(results).toMatchObject({
target: {
name: 'example',
},
topLevelDependencies: [
// expect example2 and formik as top-level deps, but not foo
expect.objectContaining({ name: 'example2' }),
expect.objectContaining({ name: 'formik' }),
],
});
});
});

describe('calculateDependenciesFromTaskGraph', () => {
Expand Down
5 changes: 4 additions & 1 deletion packages/js/src/utils/buildable-libs-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ function collectDependencies(
areTopLevelDeps = true
): { name: string; isTopLevel: boolean }[] {
(projGraph.dependencies[project] || []).forEach((dependency) => {
if (!acc.some((dep) => dep.name === dependency.target)) {
const existingEntry = acc.find((dep) => dep.name === dependency.target);
if (!existingEntry) {
// Temporary skip this. Currently the set of external nodes is built from package.json, not lock file.
// As a result, some nodes might be missing. This should not cause any issues, we can just skip them.
if (
Expand All @@ -184,6 +185,8 @@ function collectDependencies(
if (!shallow && isInternalTarget) {
collectDependencies(dependency.target, projGraph, acc, shallow, false);
}
} else if (areTopLevelDeps && !existingEntry.isTopLevel) {
existingEntry.isTopLevel = true;
}
});
return acc;
Expand Down

0 comments on commit ec38a58

Please sign in to comment.