Skip to content

Commit

Permalink
fix(core): resolve imports from linked workspace projects (#29328)
Browse files Browse the repository at this point in the history
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

When using package manager workspaces and having a dependency between
buildable projects that define their entry points in the `package.json`
to point to the outputs, the imports from source files can't be resolved
to the corresponding workspace project.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

When using package manager workspaces and having a dependency between
buildable projects that define their entry points in the `package.json`
to point to the outputs, the imports from source files should be
resolved to the corresponding workspace project.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
  • Loading branch information
leosvelperez authored Dec 13, 2024
1 parent 313f6a9 commit 657814c
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,30 @@ describe('explicit package json dependencies', () => {
type: 'lib',
data: {
root: 'libs/proj',
metadata: {
js: { packageName: 'proj', packageExports: undefined },
},
},
},
proj2: {
name: 'proj2',
type: 'lib',
data: { root: 'libs/proj2' },
data: {
root: 'libs/proj2',
metadata: {
js: { packageName: 'proj2', packageExports: undefined },
},
},
},
proj3: {
name: 'proj3',
type: 'lib',
data: { root: 'libs/proj4' },
data: {
root: 'libs/proj4',
metadata: {
js: { packageName: 'proj3', packageExports: undefined },
},
},
},
};

Expand Down Expand Up @@ -130,7 +143,7 @@ describe('explicit package json dependencies', () => {
it(`should add dependencies with mixed versions for projects based on deps in package.json and populate the cache`, async () => {
const npmResolutionCache = new Map();
const targetProjectLocator = new TargetProjectLocator(
{},
projects,
ctx.externalNodes,
npmResolutionCache
);
Expand Down Expand Up @@ -173,7 +186,7 @@ describe('explicit package json dependencies', () => {
it(`should preferentially resolve external projects found in the npmResolutionCache`, async () => {
const npmResolutionCache = new Map();
const targetProjectLocator = new TargetProjectLocator(
{},
projects,
ctx.externalNodes,
npmResolutionCache
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { dirname, join } from 'node:path';
import { DependencyType } from '../../../../config/project-graph';
import {
ProjectConfiguration,
ProjectsConfigurations,
} from '../../../../config/workspace-json-project-json';
import type { ProjectConfiguration } from '../../../../config/workspace-json-project-json';
import { defaultFileRead } from '../../../../project-graph/file-utils';
import { CreateDependenciesContext } from '../../../../project-graph/plugins';
import type { CreateDependenciesContext } from '../../../../project-graph/plugins';
import {
RawProjectGraphDependency,
type RawProjectGraphDependency,
validateDependency,
} from '../../../../project-graph/project-graph-builder';
import { parseJson } from '../../../../utils/json';
import { PackageJson } from '../../../../utils/package-json';
import type { PackageJson } from '../../../../utils/package-json';
import { joinPathFragments } from '../../../../utils/path';
import { TargetProjectLocator } from './target-project-locator';

Expand All @@ -20,40 +16,17 @@ export function buildExplicitPackageJsonDependencies(
targetProjectLocator: TargetProjectLocator
): RawProjectGraphDependency[] {
const res: RawProjectGraphDependency[] = [];
let packageNameMap = undefined;
const nodes = Object.values(ctx.projects);
Object.keys(ctx.filesToProcess.projectFileMap).forEach((source) => {
Object.values(ctx.filesToProcess.projectFileMap[source]).forEach((f) => {
if (isPackageJsonAtProjectRoot(nodes, f.file)) {
// we only create the package name map once and only if a package.json file changes
packageNameMap = packageNameMap || createPackageNameMap(ctx.projects);
processPackageJson(
source,
f.file,
ctx,
targetProjectLocator,
res,
packageNameMap
);
processPackageJson(source, f.file, ctx, targetProjectLocator, res);
}
});
});
return res;
}

function createPackageNameMap(projects: ProjectsConfigurations['projects']) {
const res = {};
for (let projectName of Object.keys(projects)) {
try {
const packageJson = parseJson(
defaultFileRead(join(projects[projectName].root, 'package.json'))
);
res[packageJson.name ?? projectName] = projectName;
} catch (e) {}
}
return res;
}

function isPackageJsonAtProjectRoot(
nodes: ProjectConfiguration[],
fileName: string
Expand All @@ -72,18 +45,19 @@ function processPackageJson(
fileName: string,
ctx: CreateDependenciesContext,
targetProjectLocator: TargetProjectLocator,
collectedDeps: RawProjectGraphDependency[],
packageNameMap: { [packageName: string]: string }
collectedDeps: RawProjectGraphDependency[]
) {
try {
const deps = readDeps(parseJson(defaultFileRead(fileName)));

for (const d of Object.keys(deps)) {
// package.json refers to another project in the monorepo
if (packageNameMap[d]) {
const localProject =
targetProjectLocator.findDependencyInWorkspaceProjects(d);
if (localProject) {
// package.json refers to another project in the monorepo
const dependency: RawProjectGraphDependency = {
source: sourceProject,
target: packageNameMap[d],
target: localProject,
sourceFile: fileName,
type: DependencyType.static,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,24 @@ describe('TargetProjectLocator', () => {
root: 'libs/parent-path/child-path',
},
},
'parent-pm-workspaces': {
name: 'parent-pm-workspaces',
type: 'lib',
data: { root: 'packages/parent-pm-workspaces' },
},
'child-pm-workspaces': {
name: 'child-pm-workspaces',
type: 'lib',
data: {
root: 'packages/child-pm-workspaces',
metadata: {
js: {
packageName: '@proj/child-pm-workspaces',
packageExports: undefined,
},
},
},
},
};
npmProjects = {
'npm:@ng/core': {
Expand Down Expand Up @@ -494,6 +512,19 @@ describe('TargetProjectLocator', () => {
);
expect(lodash4).toEqual('npm:lodash-4');
});

it('should resolve local packages linked using package manager workspaces', () => {
const targetProjectLocator = new TargetProjectLocator(
projects,
npmProjects
);
const result = targetProjectLocator.findProjectFromImport(
'@proj/child-pm-workspaces',
'packages/parent-pm-workspaces/index.ts'
);

expect(result).toEqual('child-pm-workspaces');
});
});

describe('findTargetProjectWithImport (without tsconfig.json)', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isBuiltin } from 'node:module';
import { dirname, join, posix, relative } from 'node:path';
import { clean } from 'semver';
import {
import type {
ProjectGraphExternalNode,
ProjectGraphProjectNode,
} from '../../../../config/project-graph';
Expand All @@ -10,14 +10,15 @@ import {
findProjectForPath,
} from '../../../../project-graph/utils/find-project-for-path';
import { isRelativePath, readJsonFile } from '../../../../utils/fileutils';
import { PackageJson } from '../../../../utils/package-json';
import { getPackageNameFromImportPath } from '../../../../utils/get-package-name-from-import-path';
import type { PackageJson } from '../../../../utils/package-json';
import { workspaceRoot } from '../../../../utils/workspace-root';
import { getPackageEntryPointsToProjectMap } from '../../utils/packages';
import { resolveRelativeToDir } from '../../utils/resolve-relative-to-dir';
import {
getRootTsConfigFileName,
resolveModuleByImport,
} from '../../utils/typescript';
import { getPackageNameFromImportPath } from '../../../../utils/get-package-name-from-import-path';

/**
* The key is a combination of the package name and the workspace relative directory
Expand All @@ -44,6 +45,10 @@ export class TargetProjectLocator {
private tsConfig = this.getRootTsConfig();
private paths = this.tsConfig.config?.compilerOptions?.paths;
private typescriptResolutionCache = new Map<string, string | null>();
private packageEntryPointsToProjectMap: Record<
string,
ProjectGraphProjectNode
>;

constructor(
private readonly nodes: Record<string, ProjectGraphProjectNode>,
Expand Down Expand Up @@ -135,6 +140,13 @@ export class TargetProjectLocator {
return this.findProjectOfResolvedModule(resolvedModule);
} catch {}

// fall back to see if it's a locally linked workspace project where the
// output might not exist yet
const localProject = this.findDependencyInWorkspaceProjects(importExpr);
if (localProject) {
return localProject;
}

// nothing found, cache for later
this.npmResolutionCache.set(importExpr, null);
return null;
Expand Down Expand Up @@ -242,6 +254,14 @@ export class TargetProjectLocator {
return undefined;
}

findDependencyInWorkspaceProjects(dep: string): string | null {
this.packageEntryPointsToProjectMap ??= getPackageEntryPointsToProjectMap(
this.nodes
);

return this.packageEntryPointsToProjectMap[dep]?.name ?? null;
}

private resolveImportWithTypescript(
normalizedImportExpr: string,
filePath: string
Expand Down
16 changes: 10 additions & 6 deletions packages/nx/src/plugins/js/utils/packages.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import { join } from 'node:path/posix';
import type { ProjectGraphProjectNode } from '../../../config/project-graph';
import type { ProjectConfiguration } from '../../../config/workspace-json-project-json';

export function getPackageEntryPointsToProjectMap(
projects: Record<string, ProjectConfiguration>
): Record<string, ProjectConfiguration> {
const result: Record<string, ProjectConfiguration> = {};
export function getPackageEntryPointsToProjectMap<
T extends ProjectGraphProjectNode | ProjectConfiguration
>(projects: Record<string, T>): Record<string, T> {
const result: Record<string, T> = {};
for (const project of Object.values(projects)) {
if (!project.metadata?.js) {
const metadata =
'data' in project ? project.data.metadata : project.metadata;

if (!metadata?.js) {
continue;
}

const { packageName, packageExports } = project.metadata.js;
const { packageName, packageExports } = metadata.js;
if (!packageExports || typeof packageExports === 'string') {
// no `exports` or it points to a file, which would be the equivalent of
// an '.' export, in which case the package name is the entry point
Expand Down

0 comments on commit 657814c

Please sign in to comment.