Skip to content

Fix Sequencer event loop yielding behavior while preserving file timing fix#2

Closed
Copilot wants to merge 4 commits intocopilot/fix-d528ce6b-0887-4df3-9250-3d7622167e53from
copilot/fix-810eca8e-e0a3-4bf8-b31b-f2ba4503cb81
Closed

Fix Sequencer event loop yielding behavior while preserving file timing fix#2
Copilot wants to merge 4 commits intocopilot/fix-d528ce6b-0887-4df3-9250-3d7622167e53from
copilot/fix-810eca8e-e0a3-4bf8-b31b-f2ba4503cb81

Conversation

Copy link

Copilot AI commented Sep 8, 2025

This PR addresses the comment from @TwitchBronBron in #1 about maintaining the Sequencer's event loop yielding behavior while still fixing the original issue where files added during beforeProgramValidate were not being validated.

Problem

The original fix in #1 replaced sequencer.forEach(Object.values(this.files), ...) with a simple for loop inside a .once() block:

.once(() => {
    const filesToValidate = Object.values(this.files);
    for (const file of filesToValidate) {
        // validation logic
    }
})

While this fixed the timing issue (capturing files after the beforeProgramValidate event), it broke the Sequencer's ability to yield to the event loop periodically. This is critical for the Language Server to be able to cancel long-running validations.

Solution

This PR implements the factory function approach suggested in the review comment. I added a new forEachFactory<T>(itemsFactory: () => T[], func: (item: T) => any) method to the Sequencer class that:

  1. Accepts a factory function instead of a pre-determined items array
  2. Executes the factory at runtime to get the current list of files (after beforeProgramValidate)
  3. Maintains event loop yielding by using a nested Sequencer internally

The Program validation now uses:

.forEachFactory(() => Object.values(this.files), (file) => {
    // validation logic for each file
})

Benefits

  • Preserves original functionality: Files added by plugins during beforeProgramValidate are validated
  • Maintains event loop yielding: Language Server can still cancel long validations
  • Minimal API change: Only adds one new method to Sequencer
  • Backward compatible: All existing Sequencer usage continues to work
  • Well tested: Includes comprehensive tests for the new functionality

Testing

Added comprehensive tests that verify:

  • Factory function is called at execution time (not configuration time)
  • Event loop yielding behavior is preserved
  • All existing Program validation functionality works correctly

All existing tests continue to pass, ensuring no regressions.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Address the comment in the PR you created: @iBicha/brighterscript/pull/1 Fix Sequencer event loop yielding behavior while preserving file timing fix Sep 8, 2025
Copilot AI requested a review from iBicha September 8, 2025 02:00
@iBicha iBicha closed this Sep 9, 2025
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