-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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.
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
Dependency Graph with disconnected nodes (Library to Library dependency & nested folders) #2955
Comments
Alright, I did not realize this issue might be exactly the same as mine but after looking into the example I think it is. I've run the test setup provided by @georgiee and got the same results. SetupMy Node and Win versions are strongly different to gorgiee's, so the problem seems to affect at least a large range of node and win10 versions: Win10 Pro 1903 ResultsHere are screenshots and the relevant pieces of json data received by using dep-graph --file. Flat setup works: "dependencies": {
"debug-nx-deps-problem-e2e": [...],
"debug-nx-deps-problem": [],
"a": [
{
"type": "static",
"source": "a",
"target": "b"
}
],
"b": [],
"c": [
{
"type": "static",
"source": "c",
"target": "d"
}
],
"d": []
} As soon as package d is located in libs/nested/d, dependency detection fails. "dependencies": {
"debug-nx-deps-problem-e2e": [...],
"debug-nx-deps-problem": [],
"d": [],
"a": [
{
"type": "static",
"source": "a",
"target": "b"
}
],
"b": [],
"c": []
} |
Thank you @jbjhjm for confirming this issue. The order of the json output is the direct result of the wrong dependency map so I think that's just another manifestation of the problem and not the source. I already did some debugging but I have no affected machine at my hands so it was always remotely on a colleague's machine which is pretty cumbersome. A quick check in a Windows 10 VM did not show the same effects (although I went over this pretty quickly and I should try this again). I believe the error is buried in here: nx/packages/workspace/src/core/project-graph/build-dependencies/explicit-project-dependencies.ts Lines 10 to 35 in 10282bb
Especially in the referenced Then run the graph over and over again and check your output for something conspicuous. I will check it too but I need to let it fail locally somehow. |
I might give it a try. Would love to have this fixed before continueing app & lib development |
ok, I was able to track it down. tl;dr: findProjectWithImport strategyTargetProjectLocator.findProjectWithImport function located in
the issueresolvedModule.startsWith(p.data.root) ResolvedModule is the ABSOLUTE path on windows. So startWith can NEVER start with RELATIVE p.data.root. the bugresolvedModule path should be relative. It tries to replace absolute path using utils.app_root. return resolvedModule.resolvedFileName.replace(`${app_root_1.appRootPath}/`, ''); Fix for issuein utils/app-root::pathInner(), sanitize backslashes by converting to slashes. Possible fix: exports.appRootPath = sanitize(pathInner(__dirname));
function sanitize(dir) {
return dir.replace(/\\/g,'/')
} scope issuetracked in #2982 |
Wow great analysis & conclusion @jbjhjm. That you found the I'm happy to review it or contribute a test tomorrow and I bet @vsavkin would be happy to review such a PR too. |
Hello @jbjhjm, Regarding :
I dived into the sources this morning. One addition for the wild guessing would be finding the package.json if it's publishable. This would yield the correct result.
I did a quick check where I removed the resolvedModule check entirely. The dep-graph yielded a perfect graph (because many of our packages are publishable). So this is indeed a good fallback. On the other hand, if |
Opened pull request #3028 |
Some more notes:
Related Issues linked by the PR in @rhutchison |
Update: One of the pending PRs with the fix has been merged. Let's wait for the release to see this issue being fixed 🙏 |
@georgiee Have you pulled in our latest releases? Was the issue resolved? |
Hello and thanks for your question. We couldn't upgrade to Nx 10 yet due to some weird errors with Angular CLI aorund the solution style tsconfig changes and the angular cli vs workspace.json issues. I always expected this will hinder me to upgrade the example app — I will give it a try so we can check the status of this issue. Thanks. |
@georgiee This fix actually came in during 9.x development, no need to update to 10. I updated an older version of your example repo to 9.6 and 10.1 and the dep graph appears correct in both versions. It looks like your example repo has also been updated to Nx 10 on the master branch. Are you seeing the correct graph there? Are you able to test this with your actual repo instead of just the example? I think this is resolved but wanted to check. |
This should be fixed in the latest version of 9.x and 10.x of Nx. Please open another bug if you see it again after upgrading to the latest version. |
This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context. |
Expected Behavior
I want to use subfolders is
libs/
to group my packages and I expect a correct dependency independent of being flat or nested. With a wrong dependency graph the builds with--with-deps
will always fail. So you can use thenx dep-graph
utility for a quick check without an actual build.Current Behavior
libs/
always generate a correct graph ✅libs/
generate a correct graph on OSX ✅libs/
always fails to generate a correct graph on Windows 10 Enterprise🛑 The nested dependencies are not recognized, only the flat ones.The graph will look differently depending of the bug to happen or not:
Good ✅
![osx-nested](https://user-images.githubusercontent.com/1701755/81069046-a24d9280-8ee1-11ea-89cf-3de677bfe13f.png)
Bad (Windows 10 Enterprise)🛑
![windows-nested](https://user-images.githubusercontent.com/1701755/81069048-a2e62900-8ee1-11ea-82bf-912792fa3789.png)
Failure Information (for bugs)
I think this is clearly a bug. It's working on all OSX machines but fails on Windows 10 Enterprise for many colleagues from abroad. My testing capacity is therefore limited but I captured as much information I could. I'm happy to forward other test cases if you need more information.
Steps to Reproduce
nx dep-graph
)nx run-many --target=build --projects=a,c --with-deps
)(Windows) Bug is appearing now:
5. Move
d
intolib/nested/d
and adjust all configs (or I should have used--directory
which I didn't think of)6. Create a Graph (
nx dep-graph
) which shows a danglingd
on affected systems. Check the screenshots in the Failure Logs section or in the example repository.Context
I prepared a carefully curated demo application with a Readme listing all results and tags to have your own try. I will use some of the content of the Readme to complete this issue for your convenience:
https://github.com/georgiee/nx-debug-nested-dep-graph
It's really quick to check as the only thing you need to run is the
dep-graph
utility. If the graph is wrong the build will fail. I also added a monkey patch for an internal nx util to add some more console logs. Those will reveal that the dependency is not found and therefore the entire build has a wrong baseline.Working OSX:
Failing Windows:
Failure Logs
Copied from the Readme in the example project:
https://github.com/georgiee/nx-debug-nested-dep-graph
Baseline
That's how I created the project, which means is a standard project.
Nx has no expressive verbose mode, so I prepared a debug script that will monkey patch Nx's core file
buildable-libs-utils.js
which is involved in providing utilities for the dependency graph. You will get the following output per package during a build in addition to the other logs.Context: OSX (✅)
Here the report for OSX. Everything works as expected.
Flat (✅)
Build works:
Bash Log (with debug task)
Nested (✅)
Moved
d
into subfolder.Graph looks good. The option "group by folder" is supported in the graph viewer. So it's kind of expected to introduce folders for grouping ?
Build works:
Bash Log (with debug task)
Context: Windows (🛑)
Flat (✅)
Graph is good:
Build works:
📙 Bash Log (Debug)
Nested (🛑)
Moved
d
into subfolder. Graph is wrong.Build fails:
📙 Bash Log (Debug)
Other
This errors happens in production and the only workaround for the Windows crew so far is adding the nested dependency as an Implicit Dependency to
nx.json
. I'm unsure about the implications that's why I decided to raise this issue.Related:
Thank you for your help 🙏
The text was updated successfully, but these errors were encountered: