Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.find and .filter never match directories #212

Closed
qgustavor opened this issue Sep 7, 2024 · 2 comments · Fixed by #215
Closed

.find and .filter never match directories #212

qgustavor opened this issue Sep 7, 2024 · 2 comments · Fixed by #215

Comments

@qgustavor
Copy link
Owner

Describe the bug
.find and .filter on Files never match directories.

To Reproduce

Run this code in Node X.XX (or Deno/Firefox/Chromium/Cloudflare Workers/etc):

import { Storage } from 'megajs'
const storage = await new Storage({ ... }).ready

storage.find('some directory name') // will be always null

Expected behavior

Directories should be searchable/filterable.

Additional context

The issue lies here:

    return this.children.reduce((result, entry) => {
      if (result) return result
      if (entry.children) { // <--- here
        return deep ? entry.find(query, deep) : null
      }
      return query(entry) ? entry : null
    }, null)

One option is to add a new argument "includeDirectories" which, if true, it includes directories. Or just change it to include directories anyway.

@qgustavor
Copy link
Owner Author

Options:

  1. Always match directories
  2. Match directories only when deep is false
  3. storage.find(query, deep, includeDirectories)
  4. storage.find(query, { deep, includeDirectories })

Implications:

  1. Someone trying to filter out directories would have to use function queries like this: storage.find(node => node.name === 'name' && node.directory)
  2. TOO hard to understand
  3. Two boolean arguments in the same function makes it confusing: storage.find('name', true, true) is hard to read.
  4. Seems verbose, but fine.

@qgustavor
Copy link
Owner Author

I'm going with 1: since most people name their directories differently from their files (files include dots in most cases), having to use function queries isn't really a huge issue and since .find and .filter are somewhat relative to Array.prototype equivalent functions, allowing the query to be strings or arrays and the deep argument is more like bonus than something that makes it easier to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant