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

Send user-initiated cancel event from service worker back to streamsaver #105

Closed
wants to merge 7 commits into from

Conversation

eschaefer
Copy link

@eschaefer eschaefer commented Jun 22, 2019

This solved a missing piece for me, which is to stop the writeable stream if the readable stream is cancelled by the user, by dismissing the download dialog (in FF for example).

Here's how I use it:

import streamSaver from 'streamsaver';

export const magicStreamSaver = readableStream => {
  const fileStream = streamSaver.createWriteStream('assets.zip');

  return new Promise(resolve => {
    // more optimized
    if (window.WritableStream && readableStream.pipeTo) {
      readableStream.pipeTo(fileStream).then(() => {
        console.log('done writing');

        resolve();
      });
    } else {
      window.writer = fileStream.getWriter();

      const reader = readableStream.getReader();

      const pump = async () => {
        while (true) {
          const { done, value } = await reader.read(); 

          if (streamSaver.isUserAborted) {
            break;
          }

          if (done) {
            break;
          }

          window.writer.write(value);
        }

        window.writer.close();
        resolve();
      };

      pump();
    }
  });
};

@@ -201,6 +203,9 @@
}

channel.port1.onmessage = evt => {
// Track whether readable stream was cancelled by user
streamSaver.isUserAborted = !!evt.data.userAborted;

Copy link
Owner

Choose a reason for hiding this comment

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

You should not set a global flag for all streams.
It should abort/close the writable stream

I'm not going to accept this pr unless that is changed

@@ -24,7 +25,8 @@
WritableStream: window.WritableStream || ponyfill.WritableStream,
supported: true,
version: { full: '2.0.0', major: 2, minor: 0, dot: 0 },
mitm: 'https://jimmywarting.github.io/StreamSaver.js/mitm.html?version=2.0.0'
mitm: 'https://jimmywarting.github.io/StreamSaver.js/mitm.html?version=2.0.0',
isUserAborted,
Copy link
Owner

Choose a reason for hiding this comment

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

Remove (revert)

@@ -38,6 +38,7 @@ self.onmessage = event => {
metadata[0] = evt.data.readableStream
}
} else {
port.postMessage({ userAborted: false })
Copy link
Owner

Choose a reason for hiding this comment

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

Probably would have called it just abort, cancel, aborted or cancelled. The stream can abort without user interaction like having a timed AbortController

@eschaefer
Copy link
Author

Thanks for the feedback! Will make the changes as soon as possible.

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.

2 participants