Skip to content

Conversation

@odzhychko
Copy link

The ignore pattern was applied only to the file name and not to the full file path.
This caused files that should have been ignored to appear in the key listing.

Resolves #621

The ignore pattern was applied only to the file name and not to the full file path.
This caused files that should have been ignored to appear in the key listing.

Resolves unjs#621
@odzhychko odzhychko requested a review from pi0 as a code owner April 15, 2025 19:24
}
} else {
if (!(ignore && ignore(entry.name))) {
if (!(ignore && ignore(entryPath))) {
Copy link
Member

Choose a reason for hiding this comment

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

I have a feeling some old patterns could only cover entry.name but not sure without testing. to be on safe side, we could check both.

Copy link
Author

@odzhychko odzhychko Apr 16, 2025

Choose a reason for hiding this comment

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

The fix as proposed is indeed breaking for some patterns.

For example, the pattern "ignored.txt" currently would exclude the file "{base}/dir/ignored.txt" from key listings. But after the fix, the pattern would need to be changed to "**/ignored.txt".

One could argue that excluding "{base}/dir/ignored.txt" because of the pattern "ignored.txt" is also a bug (with the same cause as #621) because this pattern does not exclude the file from being watched. The following example shows the pattern having an effect on storage.getKeys but not on storage.watch.

import { createStorage } from "unstorage";
import fsDriver fSo I think, "entry.name" could be checked for backward compatibility. 
But I guess that is something for you as a maintainer to decide. rom "unstorage/drivers/fs";
import { writeFile, mkdir } from "fs/promises";
import { join } from "path";

const testFolders = join("base", "dir");
await mkdir(testFolders, { recursive: true });

const storage = await createStorage({
  driver: fsDriver({
    base: "./base",
    ignore: [
      "ignored.txt"
    ]
  }),
});

const unwatch = await storage.watch((event, key) => {
  console.log(event, key)
});

await writeFile("./base/dir/ignored.txt", "test");
await writeFile("./base/dir/notIgnored.txt", "test");
// "update dir:notIgnored.txt" is logged.
// "update dir:ignored.txt" is not logged.


console.log(await storage.getKeys())
// "[ 'dir:notIgnored.txt' ]"
await unwatch()

Whether what I describe can be considered a bug or just an inconsistent feature worth maintaining backwards compatibility for, is something for you to decide. Please let me know what you decided, and I will make the appropriate change.

My opinion is that backwards compatibility could be maintained without impeding intended usage. Usually, patterns should always start with "**" or "/" because the ignore patterns are applied to an absolute path. But if someone nevertheless just used the file name (e.g., "ignored.txt") and it worked, the current behavior could be maintained (maybe until the next major version) by checking entry.name in addition to entryPath.

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.

Ignored files are not excluded from key listing

2 participants