refactor(watcher): effectify FileWatcher as scoped service#17601
Closed
kitlangton wants to merge 14 commits intoeffectify-watcherfrom
Closed
refactor(watcher): effectify FileWatcher as scoped service#17601kitlangton wants to merge 14 commits intoeffectify-watcherfrom
kitlangton wants to merge 14 commits intoeffectify-watcherfrom
Conversation
Convert FileWatcher from Instance.state namespace to an Effect ServiceMap.Service with proper scope-based lifecycle management.
The @parcel/watcher callback fires from a native C++ uv_async handle which does not preserve Node.js AsyncLocalStorage context. Wrap the callback in Instance.provide to restore the instance ALS context so Bus.publish can resolve instance-scoped state and directory.
Add Instance.bind(fn) which captures the current instance ALS context and returns a wrapper that synchronously restores it when called. This prevents silent failures when callbacks fire outside the instance async context (native addons, event emitters, timers). Use it in FileWatcherService to bind the @parcel/watcher callback, replacing the async Instance.provide workaround.
a9a847d to
1ff0d3b
Compare
…pace Use Effect.Config.boolean with ConfigProvider (defaults to env) so tests can override flags via ConfigProvider.layer without module mocking. Removes mock.module that was poisoning Flag getters and causing cross-test pollution in config/instruction tests.
1ff0d3b to
cb5372c
Compare
3286162 to
f5481e2
Compare
Document Instance-scoped services, Instance.bind for ALS context in native callbacks, and the Flag → Effect.Config migration pattern.
f5481e2 to
cea3f2c
Compare
daa2d73 to
3e532b2
Compare
5b04de9 to
e88538a
Compare
Probe both root and git watcher readiness before asserting events so the watcher tests stop racing startup. Also make the PTY ordering test use a deterministic short-lived process and keep native PTY callbacks bound to the instance context.
e88538a to
330f315
Compare
- Use property shorthand `{ init }` in catchCause fallback
- Make BusUpdate.directory required (non-optional)
- Use Effect.runSync instead of Effect.runFork in synchronous callback
de4b21f to
59cf7e5
Compare
The deny path is synchronous once the Effect runs, but the ManagedRuntime can take >100ms to start on slow CI runners. Just await the result directly — bun's 30s test timeout is the backstop if it ever hangs.
When w.subscribe() takes longer than SUBSCRIBE_TIMEOUT_MS and eventually resolves, the subscription was left active and unmanaged. Now the catchCause handler explicitly unsubscribes it, matching the old withTimeout cleanup behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FileWatcherfromInstance.statenamespace to an EffectServiceMap.Servicewith scope-based lifecycleFileWatcherServiceinto the per-instanceLayerMapso watchers are created/destroyed with each instanceEffect.timeoutandEffect.catchCausefor subscribe error handling instead of promise-basedwithTimeoutInstanceContextinstead of globalInstancesingletonsStacked on #17544.
Testing
bun run test test/file/watcher.test.tsbunx tsgo --noEmit