Skip to content

Create WaveEnv narrowing for WaveConfig View#3068

Merged
sawka merged 3 commits intomainfrom
copilot/add-waveenv-narrowing
Mar 16, 2026
Merged

Create WaveEnv narrowing for WaveConfig View#3068
sawka merged 3 commits intomainfrom
copilot/add-waveenv-narrowing

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

The waveconfig WaveEnv narrowing work did not need its own dedicated waveconfig.test.tsx. This change removes that file and leaves the implementation-only changes in place.

  • Scope

    • Delete frontend/app/view/waveconfig/waveconfig.test.tsx
    • Keep the waveconfig view/model WaveEnv narrowing intact
  • Result

    • Reduces test surface added for the WaveEnv refactor
    • Avoids carrying a bespoke test file for behavior already covered sufficiently elsewhere in the frontend suite
- frontend/app/view/waveconfig/waveconfig.test.tsx

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 15, 2026

Deploying waveterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 608a368
Status: ✅  Deploy successful!
Preview URL: https://98b03b47.waveterm.pages.dev
Branch Preview URL: https://copilot-add-waveenv-narrowin.waveterm.pages.dev

View logs

Copilot AI changed the title [WIP] Add WaveEnv narrowing for waveconfig files Add WaveEnv narrowing for waveconfig view and model Mar 15, 2026
Copilot AI requested a review from sawka March 15, 2026 21:38
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Copilot AI changed the title Add WaveEnv narrowing for waveconfig view and model Remove unnecessary dedicated waveconfig test Mar 15, 2026
@sawka sawka changed the title Remove unnecessary dedicated waveconfig test Create WaveEnv narrowing for WaveConfig View Mar 15, 2026
@sawka sawka marked this pull request as ready for review March 15, 2026 21:52
@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Mar 15, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This PR refactors the waveconfig view to use WaveEnv narrowing (dependency injection) instead of global imports. This follows the same pattern used elsewhere in the codebase.

Changes Summary

  • waveconfig-model.ts: Refactored to use WaveConfigEnv for all dependencies (RPC, atoms, electron API)
  • waveconfig.tsx: Added useWaveEnv hook to access environment-specific atoms
  • waveconfigenv.ts: New file defining the WaveConfigEnv type

Key Improvements

  1. Better testability: Dependencies are now injected rather than imported globally
  2. Runtime evaluation: isWindows() is now called at runtime via this.env.isWindows() instead of at module load time
  3. Consistent pattern: Follows the WaveEnv narrowing pattern used in other views
Files Reviewed (3 files)
  • frontend/app/view/waveconfig/waveconfig-model.ts - No issues
  • frontend/app/view/waveconfig/waveconfig.tsx - No issues
  • frontend/app/view/waveconfig/waveconfigenv.ts - No issues

@sawka sawka merged commit 134b388 into main Mar 16, 2026
6 checks passed
@sawka sawka deleted the copilot/add-waveenv-narrowing branch March 16, 2026 18:03
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.

2 participants