Skip to content

Conversation

@lmac-1
Copy link
Collaborator

@lmac-1 lmac-1 commented Dec 23, 2025

Description

This PR addresses some extra feedback from @stuartc in #4110:

  • Moves useMonacoSync hook from assets/js/hooks (intended for Phoenix LiveView hooks only) to assets/js/log-viewer where it's co-located with its only consumer.
  • Adds component tests that mock Monaco Editor and verify the race condition fix handles logs arriving before, during, and after editor initialisation. Tests confirm the fix works when store updates arrive instantly via WebSocket.
  • Includes monaco-editor mock infrastructure in vitest config to prevent module resolution errors during test runs.

Validation steps

  1. Follow validation steps in Fix: Run logs disappear on refresh in IDE #4110
  2. Make sure tests pass

Additional notes for the reviewer

  1. This PR is implementing some extra feedback in Fix: Run logs disappear on refresh in IDE #4110 from @stuartc
  2. It would be best if @stuartc could be one of the reviewers of this PR as it was his feedback. It's not urgent to get merged in as it's just improvements/refactoring
  3. I have not updated the changelog as this is refactoring - please let me know if this is wrong

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

Moves useMonacoSync hook from assets/js/hooks (intended for Phoenix
LiveView hooks only) to assets/js/log-viewer where it's co-located with
its only consumer.

Adds component tests that mock Monaco Editor and verify the race
condition fix handles logs arriving before, during, and after editor
initialization. Tests confirm the fix works when store updates arrive
instantly via WebSocket.

Includes monaco-editor mock infrastructure in vitest config to prevent
module resolution errors during test runs.
@github-project-automation github-project-automation bot moved this to New Issues in v2 Dec 23, 2025
@lmac-1 lmac-1 requested review from elias-ba and stuartc December 23, 2025 00:23
@lmac-1 lmac-1 changed the title Address PR #4110 review feedback: relocate hook and add tests Address PR #4110 review feedback: relocate useMonacoSync hook and add tests Dec 23, 2025
@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.31%. Comparing base (c917393) to head (aa37510).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4278      +/-   ##
==========================================
+ Coverage   89.26%   89.31%   +0.05%     
==========================================
  Files         425      425              
  Lines       19899    19899              
==========================================
+ Hits        17762    17773      +11     
+ Misses       2137     2126      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

2 participants