denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
93.41k stars 5.18k forks source link

node:fs/promises watcher polyfill is not AsyncIterable #23562

Open jldec opened 2 months ago

jldec commented 2 months ago

Version: Deno 1.42.4

The deno polyfill for node fsPromises.watch() does not appear to return an AsyncIterator as documented here for node.

With deno run, using the returned watcher with for await throws TypeError: watcher is not async iterable. The same code works with node.

$ deno run fswatch-node.js
✅ Granted read access to <CWD>.
Watching for file changes in /Users/jldec/opral/deno, press Ctrl+C to exit.
error: Uncaught (in promise) TypeError: watcher is not async iterable
  for await (const event of watcher) {
                            ^
    at run (file:///Users/jldec/opral/deno/fswatch-node.js:8:29)
    at file:///Users/jldec/opral/deno/fswatch-node.js:13:1

$ node fswatch-node.js 
Watching for file changes in /Users/jldec/opral/deno-node-fs-promises-watch, press Ctrl+C to exit.
>>>> event { eventType: 'rename', filename: 'foo.bar' }

repro

fswatch-node.js from: https://github.com/jldec/deno-node-fs-promises-watch

import process from "node:process"
import fs from "node:fs/promises"

async function run() {
  const watcher = fs.watch(process.cwd(), { recursive: true })
  console.log(`Watching for file changes in ${process.cwd()}, press Ctrl+C to exit.`);

  for await (const event of watcher) {
    console.log(">>>> event", event)
  }
}

run()

Ref: https://github.com/denoland/deno/blob/main/ext/node/polyfills/_fs/_fs_watch.ts#L126C5-L126C18

aguilera51284 commented 1 month ago

Is this issue still unreviewed? :(

BlackAsLight commented 1 month ago

Looking at the code in question it seems it would need to call .ref() on that return line https://github.com/denoland/deno/blob/main/ext/node/polyfills/_fs/_fs_watch.ts#L155, but it would be returning an async iterator for a Deno.FsEvent instead of a node one which has different property names.

sntran commented 1 week ago

I have also encountered this, and it seems that the watch in question was promisified instead of being an AsyncIterator like node:fs/promises.

await watch(path) does not fulfill the promise.