Skip to content

Resolve config items on demand so untaken references are never resolved #1158

Description

@theoephraim

Follow-up from #1153 (and() / or()).

Problem

Resolver functions short-circuit their own nested function calls (and(false, op(...)) never calls op()), but not item references. and(false, $EXPENSIVE) and if(false, $A, $B) still resolve EXPENSIVE / A, because resolveEnvValues schedules each item only after every key in its dependencyKeys has resolved, and ref() just reads the already-resolved value.

In a full load this costs nothing, since every item resolves anyway. It matters for partial loads, where the unused dependency is otherwise never needed:

  • --filter: matched items plus their transitive deps
  • early-resolved @import(..., enabled=...) conditions

Proposal: resolve items on demand

Switch the graph from a push scheduler to pull-based resolution:

  • ConfigItem.resolve() memoizes its in-flight promise so concurrent readers share one run (no double op() calls)
  • getDepValue (only used by ref()) becomes async and awaits depItem.resolve() instead of throwing when the dep is unresolved
  • resolveEnvValues becomes a Promise.all over the requested keys; deps are pulled in as they are read, still in parallel
  • earlyResolve() and the transitive-dep expansion used by --filter / @import(enabled=...) go away; those paths resolve just the keys they need
  • keep static dep collection for cycle detection, type generation, etc. Static cycles stay an error, so a runtime deadlock is impossible (runtime reads are a subset of static edges)

This makes if(), ifs(), remap(), and(), or() and fallback() lazy for references without marking conditional arg positions per function.

Things to handle

  • "Dependency X is invalid" would only apply when X is actually read, not when it sits in an untaken branch. Arguably more correct, but error output and some tests change
  • each on-demand resolve needs its own resolution context (current item, cache store) so cache hits are attributed correctly
  • ordering of side effects (exec() etc.) may shift; tests relying on order (e.g. the increment test resolver) may need updates
  • partial loads leave more items unresolved; audit code that assumes all deps of a resolved item are resolved (e.g. checkForSensitiveValuesInsideNonSensitiveOnes)

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions