oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
72.08k stars 2.58k forks source link

--hot/--watch causes fs.watch to drop filechange events #4854

Open bergkvist opened 10 months ago

bergkvist commented 10 months ago

What version of Bun is running?

1.0.0+822a00c4d508b54f650933a73ca5f4a3af9a7983

What platform is your computer?

Linux 6.5.1-1-MANJARO x86_64 unknown

What steps can reproduce the bug?

// index.ts
import fs from 'fs'
declare global { var watcher: fs.FSWatcher }
globalThis.watcher?.close()
globalThis.watcher = fs.watch('./test.txt', { recursive: true }, (event, filename) => {
  console.log('Detected event', event, filename)
})

Execute this using bun index.ts - changes are detected, and watching works perfectly well.

Now, bun --hot index.ts or bun --watch index.ts can be run instead, and the file watching becomes much less reliable. You can spam Ctrl+S in VSCode to see this unreliability more clearly

What is the expected behavior?

Using --watch or --hot shouldn't affect the reliability of fs.watch

What do you see instead?

When using --watch/--hot, fs.watch doesn't always react to file change events.

Additional information

No response

Jarred-Sumner commented 10 months ago

@cirospaciari I think we could make fs.watch support hot-reloading by only updating the handler . Since there is only one callback for fs.watch and it cannot be added later, this can be done relatively easily.

yus-ham commented 10 months ago

Facing into such this. seems currently bun --hot not destroys old FSWatcher

import {watch} from 'fs/promises'

const dir = '/data/backend/runtime/captcha'

console.info('Starting watch dir changes: '+ dir)

const watcher = watch(dir)

for await (const e of watcher) {
  console.info('hello')
}

Dir watching works. I see hello in terminal logs. but after add line change from console.info('hello') to console.info('hello', 'world) i see two lines after dir changes

hello
hello world
bergkvist commented 10 months ago

@yus-ham Yeah, you need to close the previous watcher because it is not cleaned up when hot reloading. But I believe this is expected behaviour. That's why I included this in my example:

globalThis.watcher?.close()
globalThis.watcher = ...

(globalThis persists between hot reloads)