Skip to content

refactor: extract config cache into a dedicated module with clear/reset support#292

Merged
margaretjgu merged 7 commits into
mainfrom
refactor/config-cache-module
May 21, 2026
Merged

refactor: extract config cache into a dedicated module with clear/reset support#292
margaretjgu merged 7 commits into
mainfrom
refactor/config-cache-module

Conversation

@margaretjgu

@margaretjgu margaretjgu commented May 12, 2026

Copy link
Copy Markdown
Member

Closes #210

Summary

src/cli.ts used a bare module-level let earlyConfig to cache the early config load result and share it with the preAction hook. This works fine today since cli.ts is a top-level entry point, but if the same caching pattern were used in a library module there would be no way for tests to reset the cached state between runs — stale values could leak across test cases.

This PR extracts the cache into src/config/cache.ts with a clear public API:

  • getCachedConfig() — returns the cached LoadConfigResult or undefined
  • setCachedConfig(result) — stores a result in the cache
  • clearCachedConfig() — resets the cache; intended for test cleanup

cli.ts is updated to call these instead of reading/writing the bare variable directly. Runtime behaviour is unchanged.

Changes

  • src/config/cache.ts (new) — dedicated cache module
  • src/cli.ts — replace bare let earlyConfig with getCachedConfig / setCachedConfig calls
  • test/config/cache.test.ts (new) — 7 unit tests covering all three exports (100 % line/branch/function coverage)

…et support

Replaces the bare module-level `let earlyConfig` in cli.ts with a dedicated
`src/config/cache.ts` module that exports getCachedConfig, setCachedConfig, and
clearCachedConfig. This gives tests a safe reset hook (clearCachedConfig) to
prevent stale state from leaking across test cases if the caching pattern
spreads to library modules.

Closes #210
@margaretjgu margaretjgu force-pushed the refactor/config-cache-module branch from 37589c5 to 4c72492 Compare May 12, 2026 16:22
@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

MegaLinter analysis: Success

Descriptor Linter Files Fixed Errors Warnings Elapsed time
✅ COPYPASTE jscpd yes no no 8.26s
✅ REPOSITORY gitleaks yes no no 54.45s
✅ REPOSITORY git_diff yes no no 0.64s
✅ REPOSITORY secretlint yes no no 30.99s
✅ REPOSITORY trivy yes no no 16.65s
✅ TYPESCRIPT eslint 6 0 0 4.88s

Notices

📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining SECURITY_SUGGESTIONS: false)

See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security
Show us your support by starring ⭐ the repository

@JoshMock JoshMock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simpler implementation might be to update loadConfig to take an optional cached option, which is true by default (or a refresh option that's false by default). That way, config/loader.ts can handle its own caching internally and any place that needs to ensure it gets a fresh config every time doesn't have to import a separate module.

Per JoshMock's suggestion: delete cache.ts and handle caching inside
loadConfig itself via a refresh option (default false = use cache).

- loadConfig() returns the cached result on repeat calls
- loadConfig({ refresh: true }) bypasses cache and loads fresh
- clearConfigCache() exported from loader.ts for test cleanup
- cli.ts drops getCachedConfig/setCachedConfig imports; preAction hook
  passes refresh: true when CLI flags override the defaults
- cache.test.ts rewired to test caching through loadConfig directly
@margaretjgu

Copy link
Copy Markdown
Member Author

Good suggestion, done. Deleted cache.ts and moved everything into loadConfig itself:

  • loadConfig() returns the cached result on repeat calls (default behavior)
  • loadConfig({ refresh: true }) bypasses the cache and loads fresh, updating the cache for subsequent calls
  • clearConfigCache() exported from loader.ts for test cleanup between cases

cli.ts no longer imports a separate cache module. The preAction hook passes refresh: true when any CLI flag overrides are present (--config-file, --use-context, --command-profile), and defaults to the cached path otherwise.

cache.test.ts is rewritten to test caching behavior through loadConfig directly using a real temp config file.

@margaretjgu margaretjgu enabled auto-merge (squash) May 21, 2026 14:26
@margaretjgu margaretjgu merged commit c2093e2 into main May 21, 2026
19 checks passed
@margaretjgu margaretjgu deleted the refactor/config-cache-module branch May 21, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: extract config cache into a dedicated module with clear/reset support

2 participants