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

Support using AbortController with FileAPI #159

Open
benjamingr opened this issue Nov 20, 2020 · 6 comments
Open

Support using AbortController with FileAPI #159

benjamingr opened this issue Nov 20, 2020 · 6 comments

Comments

@benjamingr
Copy link

Hey,

It would be useful to use the web platform standard cancellation method to abort readers.
Other web platform code (like fetch) supports cancellation through signals.

Code before:

async function readFile(file, { signal }) {
  var reader = new FileReader();
  const handler = e => reader.abort();
  signal.addEventListener('abort', handler, { once: true });
  reader.addEventListener('load', () => signal.removeEventListener('abort', handler));
  reader.addEventListener('error', () => signal.removeEventListener('abort', handler));
  const p = new Promise((resolve, reject) => {
    reader.onload = (e) => resolve(e.target.result);
    reader.onerror = (e) => reject(reader.error);
  });
  reader.readAsText(file);
  return await p;
}

Possible suggested change, assuming signal support:

async function readFile(file, { signal }) {
  var reader = new FileReader({ signal });
  const p = new Promise((resolve, reject) => {
    reader.onload = (e) => resolve(e.target.result);
    reader.onerror = (e) => reject(reader.error);
  });
  reader.readAsText(file);
  return await p;
}
@domenic
Copy link
Contributor

domenic commented Nov 20, 2020

Not speaking for the editors here, but for my part, I consider FileReader deprecated and replaced by the .stream(), .text(), and .arrayBuffer() methods.

@benjamingr
Copy link
Author

@domenic Ah thanks, so in that case the ask should be to support signals in .text and .arrayBuffer?

Is it currently possible to abort an ongoing .text()?

Since it returns a promise and does not take a signal parameter I don't see/know how to do that.

@domenic
Copy link
Contributor

domenic commented Nov 20, 2020

Yeah, that would make sense to me. Indeed they cannot be aborted currently; you'd need to use .stream() + TextDecoderStream.

@annevk
Copy link
Member

annevk commented Nov 20, 2020

It would be a bit unfortunate to lose the symmetry with Request/Response, but maybe that's okay. I guess these are already different in that you can invoke them multiple times.

@domenic
Copy link
Contributor

domenic commented Nov 20, 2020

I mean, we could also add it to Request / Response's methods. Aborting in that case would probably throw away the entire body.

@mkruisselbrink
Copy link
Collaborator

Adding an AbortController to .text() and .arrayBuffer() seems reasonable to me. Agreed that I'd consider FileReader deprecated/legacy, especially now all major browsers support the new methods.

I agree that adding it to Request/Response (i.e. Body) as well would be ideal, but I don't think it would be too bad to only have it here either. It would still be true that you can do anything with these methods on Blob that you can do on other Bodys. And the reverse isn't true already.

@w3c w3c deleted a comment from ayalatech Dec 1, 2023
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

4 participants