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

Make filter async #55

Open
lpatiny opened this issue Nov 11, 2022 · 4 comments
Open

Make filter async #55

lpatiny opened this issue Nov 11, 2022 · 4 comments
Assignees

Comments

@lpatiny
Copy link
Member

lpatiny commented Nov 11, 2022

Currently the filter function is similar to the one on array:

filter(callback: (file: FileCollectionItem) => unknown) {
return new FileCollection(this.files.filter(callback));
}

I would make it async in order to be able to filter on the content of the files.

@targos ok for you ?

@lpatiny lpatiny self-assigned this Nov 11, 2022
@targos
Copy link
Member

targos commented Nov 11, 2022

I'm not really ok with this, at least not until we have a system to avoid fetching the same file multiple times.

@lpatiny
Copy link
Member Author

lpatiny commented Nov 11, 2022

Ok. This is also related to the groupBy callback that should also be async if we want to be able to group by the content of the file.

I would have expect that the browser would cache the value but this is indeed not the case

<html>
  <head>
    <script src="../dist/filelist-utils.js"></script>
  </head>
  <body>
    <script>
      async function doAll() {
        const fileCollection = await FileListUtil.fileCollectionFromWebservice(
          'https://zakodium-oss.github.io/analysis-dataset/gcms.json',
        );

        for (let i = 0; i < 10; i++) {
          console.log(await fileCollection.files[0].text());
          console.log(await fileCollection.files[0].arrayBuffer());
          await delay(5000);
        }
      }

      function delay(ms) {
        return new Promise((resolve) => setTimeout(resolve, ms));
      }

      doAll();
    </script>
    FileList
  </body>
</html>

image

@targos
Copy link
Member

targos commented Nov 11, 2022

we can keep this open if you want but I think it deserves a discussion/proposal before implementing anything in this library.

@lpatiny
Copy link
Member Author

lpatiny commented Nov 11, 2022

Requires to solve this issue:

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

No branches or pull requests

2 participants