fix(core): fix(core): abide by .nxignore when hashing files

[yharaskrik]

ISSUES CLOSED: #3897

@vsavkin Not sure if we would rather have implicit deps take precedent over .nxignore

With this PR if workspace files are listed in .nxignore they will be ignored unless they are also listed in implicitDependencies

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit a927722. Click to see the status, the terminal output, and the build insights.

Status	Command	Start Time
	nx build-base express	10/19/2020, 10:15:31 PM
	nx build-base nest	10/19/2020, 10:15:31 PM
	nx build-base next	10/19/2020, 10:15:36 PM
	nx build-base react	10/19/2020, 10:15:31 PM
	nx run-many --target=build --all --parallel	10/19/2020, 10:14:57 PM
	nx run-many --target=e2e --projects=e2e-angular,e2e-bazel	10/19/2020, 10:15:46 PM
	nx run-many --target=e2e --projects=e2e-cypress,e2e-jest,e2e-nx-plugin	10/19/2020, 10:16:11 PM
	nx run-many --target=e2e --projects=e2e-next	10/19/2020, 10:16:08 PM
	nx run-many --target=e2e --projects=e2e-node	10/19/2020, 10:15:38 PM
	nx run-many --target=e2e --projects=e2e-react	10/19/2020, 10:15:39 PM
	nx run-many --target=e2e --projects=e2e-web,e2e-linter,e2e-storybook	10/19/2020, 10:16:26 PM
	nx run-many --target=e2e --projects=e2e-workspace	10/19/2020, 10:16:20 PM
	nx run-many --target=lint --all --parallel	10/19/2020, 10:18:15 PM
	nx run-many --target=test --all --parallel	10/19/2020, 10:15:02 PM

---

Sent with 💌 from NxCloud.

[vsavkin]

Thank you for submitting this PR.

Let's step back a bit and look at the original issue. You would like not invalidate the cache for all projects when you add a new library. Excluding nx.json and tsconfig.base.json will achieve that but will also result in legitimate cache invalidations to get undetected. I don't think this is desirable. Perphaps a more intelligent hasing of nx.json could be a solution. But computation caching is more conservative than "affected:*". There is no way to know when the computatoin caching didn't work correctly, so it has to be correct every time.

The purpose of .nxignore is to list the files that have to be checked in, but aren't something Nx should worry about. Nx has to know about global config files, so nxignoring them doesn't seem right.

Does it make sense?

[yharaskrik]

@vsavkin that makes perfect sense!

I guess this was a rudimentary solution to a larger issue at hand that I laid out in #3897

Really the may issue that I am trying to solve I the massive cache invalidation that can happen in certain cases.

Let's look at a "worst-case scenario", we have a repo that has hundreds (or thousands) of projects in it.

I make a change to lib1 that has far-reaching implications as it is a utility library used in many places, naturally, this would cause cache misses and make for large rebuilds/retests. So I push my changes and let the CI do it's thing. Everything passes and I get the green light, except, while I was doing that a different PR got merged into the base branch that had generated a new library but doesn't get used anywhere and doesn't functionally affect anything (thus changing nx.json, tsconfig.*.json and angular.json). I now have a conflict and have to rebase, but when I push because those json files have changed now every test and build I had to run prior is cache miss after cache miss, even though there were no actual changes and logically I should be able to reuse the cache from before.

This is especially an issue when I am having issues described in this thread, our jest test suites can take upwards of 15-20s per suite (which causes massive CI times), currently, everyone that has tried to help me track it down (@Cammisuli included). Because of the huge cache misses we can get when our json files change we are rebuilding and retesting so much code. This would be much less of an issue if our test suites took a reasonable amount of time, there is clearly a performance issue somewhere else causing it I just have not been able to track it down.

Another case if we look at the contrived example above is that once the json files change then if we are using --with-deps to rebuild our projects with dependencies now all of those build artifacts have their caches broken as well even though the libraries cached code wouldn't have changed.

I think a more intelligent caching solution is definitely in need of here, my proposal would be that when calculating the hash for a library, instead of hashing the entire nx.json, tsconfig.*.json and angular.json combine each record from each from that libraries dep tree and hash those.

Example:

lib1 depends on lib2 but not lib3

When hashing lib1 -> the hash would consist of
nx.json['projects']['lib1'] && nx.json['projects']['lib2'] &&
tsconfig.*.json['paths']['lib1'] && tsconfig.*.json['paths']['lib2'] && <other compiler options from this file> &&
angular.json['projects']['lib1'] && angular.json['projects']['lib2']

This would ensure that generating libraries or changing tags on other libraries or anything else that doesn't directly related functionality to the project whos hash is currently being calculated will not be accounted for in the hash, since I think we can safely say that those projects configurations shouldn't be accounted for if the project doesn't exist in the current projects dependency tree.

I hope that I was clear in the problem I am facing and the potential solution and that I didn't ramble too much).

More than happy to have a deeper conversation around it and contribute where I am.

[vsavkin]

I agree that reducing the number of times we hit the worst case scenario is super important. And right now it's often caused by modifying nx.json, workspace.json (or angular.json), and tsconfig.base.json. Global configuration files also cause headaches when rebassing PRs. 20k of JSON is not easy to quickly glance through.

The solution we considered:

1. Having a project.json for each project, so only global things will be stored in the global config.
2. Transforming nx.json and workspace.json to exclude the projects that aren't in the transitive closure of deps. We already transform files some before we feed them into the caching mechanism.
3. Doing both :)

Option 2 is I think what will solve your problem. implicitDepsHash in Hasher is where the change will have to be made. As you can see hashFile takes a transformer. So the hasher could pass a transformer to handle nx.json, workspace.json and tsconfig.base.json in a special way to exclude irrelevant projects and path mappings. The paths mappings is the trickiest thing to handle.

If you are interested in giving it a try, I'd be happy to review it. If not, which is totally OK, one of the core team members will likely to do it right after V11 release.

[yharaskrik]

Thanks for the explanation Victor!

Took me a while the parse through the Hasher the first time but I don't have any plans today so I'll take another stab at it and see what I can come up with based in #2.

[yharaskrik]

Closing this in favour of #4055

[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.