refactor(project): Refresh dependency indices on dependency-set change#1469
Open
RandomByte wants to merge 3 commits into
Open
refactor(project): Refresh dependency indices on dependency-set change#1469RandomByte wants to merge 3 commits into
RandomByte wants to merge 3 commits into
Conversation
validateCache only calls _refreshDependencyIndices in the RESTORING_DEPENDENCY_INDICES state when #changedDependencyResourcePaths is non-empty. That accumulator is populated exclusively by dependencyResourcesChanged(), which fires when a dependency *resource* changes. When the dependency *set* changes across builds (a transitive library added to or removed from the project graph), no resource is reported, the accumulator stays empty, and the refresh is skipped. The restored dependency index keeps reflecting the previous build's dependency set, so a task that globs dependency resources (buildThemes over available libraries is the prominent case, but any dependency-globbing task qualifies) processes the stale set while the result cache still reports a hit. Add three test.failing cases that seed a persisted dependency-request cache recorded against an old dependency set, then validate against a reader exposing the changed set, asserting the dependency-index signature converges on the value a full refresh produces. Two cover buildThemes (added and removed library); the third uses a generic JS glob to show the defect is not theme-specific. They are marked test.failing so CI stays green and flips to a hard error once the fix lands, prompting removal of the marker.
Tasks that glob dependency resources over all available libraries (buildThemes globs /resources/**/themes/**/*.less across every library, but any dependency-globbing task qualifies) go stale when a transitive dependency is added to or removed from the graph between two builds. No resource changed, so dependencyResourcesChanged() reports nothing and #changedDependencyResourcePaths stays empty. The RESTORING_DEPENDENCY_INDICES branch refreshed the indices only on a non-empty accumulator, so it skipped the refresh: the restored indices kept the previous build's dependency set, the result signature still matched, and the project was served from cache. Persist a dependency-set identity with each project's source index and compare it at the next build. The identity is a sha256 over the sorted ids of the transitive dependency closure, computed in ProjectBuildContext from the graph and passed into validateCache, so ProjectBuildCache compares two opaque strings and never touches the graph. On mismatch, run the full _refreshDependencyIndices even without a propagated resource change; on match, keep the fast path. The closure comes from graph.getTransitiveDependencies (mapped to ids, since name and id can differ under npm aliasing), not the task-scoped required set: getRequiredDependencies() returns only direct dependencies for standard tasks (the reader expands transitively on its own) and flips all-or-nothing with task selection. The closure matches what the reader exposes and is stable across task selections. The identity is stored as availableDependencies in the existing source index_cache row, next to tasks. #prepareSourceIndex now also writes the row when only the identity changed, since a dependency-set change does not touch source files and would otherwise leave the persisted identity stale. Same-path version swaps stay out of scope: a dependency's version is not part of the consuming project's build signature, so that case is already stale for the same structural reason and is handled separately if needed.
The three cases were marked test.failing while the defect was unfixed. validateCache now refreshes the dependency indices on a dependency-set change, so drop the .failing markers. Seed the persisted source index with an availableDependencies identity for the previous build's dependency set, and pass a differing identity into validateCache to model the added or removed transitive dependency. The tests still compare the dependency-index signature against the value a full refresh produces (computed independently via ResourceRequestManager).
RandomByte
force-pushed
the
fix/dependency-set-change-cache-staleness
branch
from
July 22, 2026 13:10
d02cec9 to
fc44204
Compare
RandomByte
marked this pull request as ready for review
July 22, 2026 13:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Tasks that glob dependency resources over all available libraries go stale when the dependency set changes between two builds.
buildThemesglobs/resources/**/themes/**/*.lessacross every library, but any dependency-globbing task qualifies.When a transitive dependency is added to or removed from the graph, no dependency resource changed, so
dependencyResourcesChanged()reports nothing and#changedDependencyResourcePathsstays empty. TheRESTORING_DEPENDENCY_INDICESbranch invalidateCacherefreshed the indices only on a non-empty accumulator, so it skipped the refresh: the restored indices kept the previous build's dependency set, the result signature still matched, and the project was served from cache with a set that no longer matched the graph.Fix
Persist a dependency-set identity with each project's source index and compare it at the next build:
sha256over the sorted ids of the project's transitive dependency closure.ProjectBuildContextfrom the graph and passed intovalidateCache, soProjectBuildCachecompares two opaque strings and never touches the graph._refreshDependencyIndicesruns even without a propagated resource change. On match, the fast path is unchanged.availableDependenciesin the existing sourceindex_cacherow, next totasks.#prepareSourceIndexalso writes the row when only the identity changed, since a dependency-set change does not touch source files.The closure comes from
graph.getTransitiveDependencies(mapped to ids, since name and id can differ under npm aliasing), not the task-scoped required set:getRequiredDependencies()returns only direct dependencies for standard tasks (the reader expands transitively on its own) and flips all-or-nothing with task selection. The closure matches what the reader exposes and is stable across task selections.Out of scope
Same-path version swaps: a dependency's version is not part of the consuming project's build signature, so that case is already stale for the same structural reason and is handled separately if it matters in practice.
Tests
Three regression cases (added library, removed library, generic non-theme JS glob), committed first as
test.failingto reproduce the defect, then activated by the fix. Full@ui5/projectunit suite passes (3331 passed, 3 skipped).