Skip to content

Conversation

sheetalkamat
Copy link
Member

No description provided.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors module resolution watching by extracting lookup-glob generation into standalone functions, introduces concurrency-safe watcher updates with sync.RWMutex, and runs watcher updates and typing-location watches in goroutines.

  • Introduced getModuleResolutionWatchGlobs and generic extractLookups in watch.go
  • Added watchMu sync.RWMutex to guard watcher updates in project.go and made updateWatchers asynchronous
  • Spawned goroutines for updateWatchers and WatchTypingLocations to decouple from the main update loop

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/project/watch.go New helper functions to gather module-resolution watch patterns
internal/project/project.go Added watchMu, updated updateWatchers signature and concurrency, adjusted locks in WatchTypingLocations and event handler
internal/project/ata.go Changed discoverAndInstallTypings to invoke WatchTypingLocations asynchronously
internal/module/types.go Added GetLookupLocations methods to resolved-module types
internal/compiler/program.go Exposed GetResolvedTypeReferenceDirectives on Program
internal/compiler/parsetask.go Added flag for automatic-type parsing in parseTask
internal/compiler/fileloader.go Integrated automatic-type-directive tasks into parsing workflow
Comments suppressed due to low confidence (3)

internal/project/project.go:354

  • [nitpick] The parameter name program shadows p.program; consider renaming it (e.g., prog) to reduce confusion.
func (p *Project) updateWatchers(ctx context.Context, program *compiler.Program) {

internal/project/watch.go:368

  • These new functions (getModuleResolutionWatchGlobs and extractLookups) introduce nontrivial logic; consider adding unit tests to verify correct extraction of lookup globs.
func getModuleResolutionWatchGlobs(program *compiler.Program) (failedLookups map[tspath.Path]string, affectingLocaions map[tspath.Path]string) {

internal/project/project.go:170

  • The code introduces sync.RWMutex but there is no sync import; add import "sync" to avoid a compile error.
	watchMu                 sync.RWMutex

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

The file loader code seems correct to me, though I am unsure about the concurrency stuff in the Project given our ongoing logical races.

I assume no tests change with the loader change since the main effect here is to actually keep track of these files for watching?

@jakebailey
Copy link
Member

Is this still needed after #1505?

@sheetalkamat sheetalkamat deleted the watchTypeRefs branch September 8, 2025 18:17
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