fix(core): fix module resolution in project locator

[rhutchison]

On windows, there is a bug in project module resolution, caused by path separator. TypeScript resolveModuleName will always return a posix normalized path, so our appRootPath needs to be normalized for string replacement.

I came across the issue when trying to run dep-graph, but other areas (ex: affected:*) which rely on TargetProjectLocator are going to be impacted.

findProjectWithImport is using typescript utils to resolve modules by import expressions. appRootPath on windows is using the windows separator \, while TypeScript resolveModuleName is returning a module with resolvedFileName containing posix separator /.

Since (angular.json) project root path's are relative, the condition is not met, and it relies on npmScope -

nx/packages/workspace/src/core/target-project-locator.ts

Line 34 in b55a1a9

if (resolvedModule && resolvedModule.startsWith(p.data.root)) {

Current Behavior (This is the behavior we have today, before the PR is merged)

resolveModuleByImport returns project files with absolute path.

Expected Behavior (This is the new behavior we can expect after the PR is merged)

resolveModuleByImport should return project files with relative path (appRootPath removed).

Issue

I have not tested, but these might be related:

#2472

#2820

[FrozenPandaz]

Thanks for the fix!

I think this normalization should happen in https://github.com/nrwl/nx/blob/master/packages/workspace/src/utils/app-root.ts#L4 (as dictated by a todo comment by @vsavkin 😅.)

Can you please look into moving the normalization there so that the path is normalized for every usage of appRootPath?

[rhutchison]

I think this normalization should happen in https://github.com/nrwl/nx/blob/master/packages/workspace/src/utils/app-root.ts#L4 (as dictated by a todo comment by @vsavkin 😅.)

Can you please look into moving the normalization there so that the path is normalized for every usage of appRootPath?

I did come across that, but I feel a change there would be too risky. The Windows -> POSIX separator replacement is only necessary to do string replacement. If we normalize appRoot to use POSIX separator everywhere, there might be further implications for the rest of the app. @FrozenPandaz @vsavkin thoughts?

[rhutchison]

Resolves #2955

Closes #3028

[FrozenPandaz]

Thank you for your contribution! LGTM!

[github-actions]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.