From c774aa49381e601214705dad5ae0eda060f59aed Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 17:53:27 +0200 Subject: [PATCH 1/7] fix: `rename` watch event missing --- cli/tsc/dts/lib.deno.ns.d.ts | 9 ++++++++- ext/node/polyfills/_fs/_fs_watch.ts | 4 +++- runtime/ops/fs_events.rs | 9 ++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/cli/tsc/dts/lib.deno.ns.d.ts b/cli/tsc/dts/lib.deno.ns.d.ts index cbf86eec66a231..c6c4ae29829b93 100644 --- a/cli/tsc/dts/lib.deno.ns.d.ts +++ b/cli/tsc/dts/lib.deno.ns.d.ts @@ -4149,7 +4149,14 @@ declare namespace Deno { * @category File System */ export interface FsEvent { /** The kind/type of the file system event. */ - kind: "any" | "access" | "create" | "modify" | "remove" | "other"; + kind: + | "any" + | "access" + | "create" + | "modify" + | "rename" + | "remove" + | "other"; /** An array of paths that are associated with the file system event. */ paths: string[]; /** Any additional flags associated with the event. */ diff --git a/ext/node/polyfills/_fs/_fs_watch.ts b/ext/node/polyfills/_fs/_fs_watch.ts index acdaff800b2c08..3965ba29a952c5 100644 --- a/ext/node/polyfills/_fs/_fs_watch.ts +++ b/ext/node/polyfills/_fs/_fs_watch.ts @@ -361,13 +361,15 @@ class FSWatcher extends EventEmitter { } } -type NodeFsEventType = "rename" | "change"; +type NodeFsEventType = "rename" | "change" | "rename"; function convertDenoFsEventToNodeFsEvent( kind: Deno.FsEvent["kind"], ): NodeFsEventType { if (kind === "create" || kind === "remove") { return "rename"; + } else if (kind === "rename") { + return "rename"; } else { return "change"; } diff --git a/runtime/ops/fs_events.rs b/runtime/ops/fs_events.rs index 367866162fcc07..8596f547dbdb81 100644 --- a/runtime/ops/fs_events.rs +++ b/runtime/ops/fs_events.rs @@ -14,6 +14,7 @@ use deno_core::op2; use deno_permissions::PermissionsContainer; use notify::event::Event as NotifyEvent; +use notify::event::ModifyKind; use notify::Error as NotifyError; use notify::EventKind; use notify::RecommendedWatcher; @@ -71,7 +72,13 @@ impl From for FsEvent { EventKind::Any => "any", EventKind::Access(_) => "access", EventKind::Create(_) => "create", - EventKind::Modify(_) => "modify", + EventKind::Modify(modify_event) => match modify_event { + ModifyKind::Name(_) => "rename", + ModifyKind::Any + | ModifyKind::Data(_) + | ModifyKind::Metadata(_) + | ModifyKind::Other => "modify", + }, EventKind::Remove(_) => "remove", EventKind::Other => "other", }; From ecb0531da889593bc03124e3cb212b6df9df1d9c Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 18:12:03 +0200 Subject: [PATCH 2/7] fix: undo duplicated type --- ext/node/polyfills/_fs/_fs_watch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/node/polyfills/_fs/_fs_watch.ts b/ext/node/polyfills/_fs/_fs_watch.ts index 3965ba29a952c5..eb2cd8bfa271e7 100644 --- a/ext/node/polyfills/_fs/_fs_watch.ts +++ b/ext/node/polyfills/_fs/_fs_watch.ts @@ -361,7 +361,7 @@ class FSWatcher extends EventEmitter { } } -type NodeFsEventType = "rename" | "change" | "rename"; +type NodeFsEventType = "rename" | "change"; function convertDenoFsEventToNodeFsEvent( kind: Deno.FsEvent["kind"], From d5b1eef571d20e42904b5b2e92963ee40ab2f98d Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 19:18:52 +0200 Subject: [PATCH 3/7] chore: add test --- runtime/ops/fs_events.rs | 2 +- tests/unit/fs_events_test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/runtime/ops/fs_events.rs b/runtime/ops/fs_events.rs index 8596f547dbdb81..58fe9d5fd2adb3 100644 --- a/runtime/ops/fs_events.rs +++ b/runtime/ops/fs_events.rs @@ -72,7 +72,7 @@ impl From for FsEvent { EventKind::Any => "any", EventKind::Access(_) => "access", EventKind::Create(_) => "create", - EventKind::Modify(modify_event) => match modify_event { + EventKind::Modify(modify_kind) => match modify_kind { ModifyKind::Name(_) => "rename", ModifyKind::Any | ModifyKind::Data(_) diff --git a/tests/unit/fs_events_test.ts b/tests/unit/fs_events_test.ts index 4f7cdc4d5a6260..783a6754f1c214 100644 --- a/tests/unit/fs_events_test.ts +++ b/tests/unit/fs_events_test.ts @@ -70,6 +70,33 @@ Deno.test( }, ); +Deno.test( + { permissions: { read: true, write: true } }, + async function watchFsRename() { + const testDir = await makeTempDir(); + const iter = Deno.watchFs(testDir); + + // Asynchronously capture two fs events. + const eventsPromise = getTwoEvents(iter); + + // Make some random file system activity. + const file = testDir + "/file.txt"; + await Deno.writeTextFile(file, "hello"); + + await Deno.rename(file, testDir + "/file2.txt"); + + // We should have gotten two fs events. + const events = await eventsPromise; + assert(events.length >= 2); + assert(events[0].kind == "create"); + assert(events[0].paths[0].includes(testDir)); + assert(events[1].kind == "rename"); + assert(events[1].paths[0].includes(testDir)); + assert(events[2].kind == "modify"); + assert(events[2].paths[0].includes(testDir)); + }, +); + // TODO(kt3k): This test is for the backward compatibility of `.return` method. // This should be removed at 2.0 Deno.test( From a21c69ba2f26e76e3106c55366f50c0cb9064188 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 19:31:44 +0200 Subject: [PATCH 4/7] chore: use assertEquals --- tests/unit/fs_events_test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/fs_events_test.ts b/tests/unit/fs_events_test.ts index 783a6754f1c214..bfaf34bfafe510 100644 --- a/tests/unit/fs_events_test.ts +++ b/tests/unit/fs_events_test.ts @@ -88,11 +88,11 @@ Deno.test( // We should have gotten two fs events. const events = await eventsPromise; assert(events.length >= 2); - assert(events[0].kind == "create"); + assertEquals(events[0].kind, "create"); assert(events[0].paths[0].includes(testDir)); - assert(events[1].kind == "rename"); + assertEquals(events[1].kind, "rename"); assert(events[1].paths[0].includes(testDir)); - assert(events[2].kind == "modify"); + assertEquals(events[2].kind, "modify"); assert(events[2].paths[0].includes(testDir)); }, ); From 833d28c78f22df106951fba1aaf5ef05b6b8180a Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 20:19:13 +0200 Subject: [PATCH 5/7] chore: debug --- tests/unit/fs_events_test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/fs_events_test.ts b/tests/unit/fs_events_test.ts index bfaf34bfafe510..0509334c8f25a5 100644 --- a/tests/unit/fs_events_test.ts +++ b/tests/unit/fs_events_test.ts @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +import console from "node:console"; import { assert, assertEquals, assertThrows, delay } from "./test_util.ts"; // TODO(ry) Add more tests to specify format. @@ -87,6 +88,7 @@ Deno.test( // We should have gotten two fs events. const events = await eventsPromise; + console.log(events); assert(events.length >= 2); assertEquals(events[0].kind, "create"); assert(events[0].paths[0].includes(testDir)); From 253c7cee040799400f5f20bcbfabf3304dfb1963 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 20:20:00 +0200 Subject: [PATCH 6/7] fix: import --- tests/unit/fs_events_test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/fs_events_test.ts b/tests/unit/fs_events_test.ts index 0509334c8f25a5..bbcd6b4bd6c61d 100644 --- a/tests/unit/fs_events_test.ts +++ b/tests/unit/fs_events_test.ts @@ -1,6 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import console from "node:console"; import { assert, assertEquals, assertThrows, delay } from "./test_util.ts"; // TODO(ry) Add more tests to specify format. From 0ee743b06f369cb7675139730f9e93b6cbde22c3 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 5 Aug 2024 21:35:23 +0200 Subject: [PATCH 7/7] fix: test --- tests/unit/fs_events_test.ts | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/tests/unit/fs_events_test.ts b/tests/unit/fs_events_test.ts index bbcd6b4bd6c61d..3a867f07ee2ab4 100644 --- a/tests/unit/fs_events_test.ts +++ b/tests/unit/fs_events_test.ts @@ -74,27 +74,19 @@ Deno.test( { permissions: { read: true, write: true } }, async function watchFsRename() { const testDir = await makeTempDir(); - const iter = Deno.watchFs(testDir); - - // Asynchronously capture two fs events. - const eventsPromise = getTwoEvents(iter); - - // Make some random file system activity. + const watcher = Deno.watchFs(testDir); + async function waitForRename() { + for await (const event of watcher) { + if (event.kind === "rename") { + break; + } + } + } + const eventPromise = waitForRename(); const file = testDir + "/file.txt"; await Deno.writeTextFile(file, "hello"); - await Deno.rename(file, testDir + "/file2.txt"); - - // We should have gotten two fs events. - const events = await eventsPromise; - console.log(events); - assert(events.length >= 2); - assertEquals(events[0].kind, "create"); - assert(events[0].paths[0].includes(testDir)); - assertEquals(events[1].kind, "rename"); - assert(events[1].paths[0].includes(testDir)); - assertEquals(events[2].kind, "modify"); - assert(events[2].paths[0].includes(testDir)); + await eventPromise; }, );