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

Canceled batch is processed the next time I choose a file #472

Closed
mgrunberg opened this issue Feb 13, 2023 · 5 comments · Fixed by #474
Closed

Canceled batch is processed the next time I choose a file #472

mgrunberg opened this issue Feb 13, 2023 · 5 comments · Fixed by #474
Assignees
Labels
bug Something isn't working completed fixed

Comments

@mgrunberg
Copy link

Describe the bug

I'm trying to prevent users from uploading more than N files. I'm using UPLOADER_EVENTS.BATCH_ADD listener and returning false when batch.items.length is greater than the limit. I'm also using UploadPreview component to display progress.

When I choose N + 1 files batch is canceled properly, no new preview items, no uploads. Then I choose less than N files and I see the previous N + 1 batch added to the preview list and uploaded.

To Reproduce
Steps to reproduce the behavior:

  1. Go to my example
  2. Click on Upload
  3. Choose 3 files
  4. Click on Upload
  5. Choose 2 files
  6. Wait 5 seconds

Expected behavior

Files from step #3 should not be uploaded, only files from step #5 should.

Versions

@rpldy version 1.3.1
Chrome version 110.0.5481.77 (Official Build) (64-bit)

Code

import Uploady, { UPLOADER_EVENTS } from '@rpldy/uploady';
import UploadButton from '@rpldy/upload-button';
import { getMockSenderEnhancer } from '@rpldy/mock-sender';
import UploadPreview from '@rpldy/upload-preview';

const mockSenderEnhancer = getMockSenderEnhancer({
  delay: 500,
});

const listeners = {
  [UPLOADER_EVENTS.BATCH_ADD]: (batch) => {
    return batch.items.length < 2;
  },
};

export default function App() {
  return (
    <Uploady
      enhancer={mockSenderEnhancer}
      listeners={listeners}
      maxConcurrent={3}
    >
      <div className="files">
        <UploadPreview rememberPreviousBatches />
      </div>
      <UploadButton />
    </Uploady>
  );
}

You can run it here

@yoavniran
Copy link
Collaborator

thanks @mgrunberg
Thats surprising but definitely looks like a bug.
I will investigate

@yoavniran yoavniran self-assigned this Feb 14, 2023
@yoavniran yoavniran added the bug Something isn't working label Feb 14, 2023
@mgrunberg
Copy link
Author

@yoavniran thanks for your quick response.

I found a workaround: listen to batch cancelation and call abort.
I can do this because my validations are the only source of batch cancelations.

import Uploady, { UPLOADER_EVENTS, useAbortBatch, useBatchCancelledListener } from '@rpldy/uploady';
import UploadButton from '@rpldy/upload-button';
import { getMockSenderEnhancer } from '@rpldy/mock-sender';
import UploadPreview from '@rpldy/upload-preview';

const mockSenderEnhancer = getMockSenderEnhancer({
  delay: 500,
});

function Workaround() {
  const abortBatch = useAbortBatch();

  useBatchCancelledListener((batch) => {
    abortBatch(batch.id)
  });

  return null;
}

const listeners = {
  [UPLOADER_EVENTS.BATCH_ADD]: (batch) => {
    return batch.items.length < 2;
  },
};

export default function App() {
  return (
    <Uploady
      enhancer={mockSenderEnhancer}
      listeners={listeners}
      maxConcurrent={3}
    >
      <div className="files">
        <UploadPreview rememberPreviousBatches />
        <Workaround />
      </div>
      <UploadButton />
    </Uploady>
  );
}

I hope this helps in the investigation.

@yoavniran
Copy link
Collaborator

thanks. its actually a very simple bug and yet surprising that nor I or anyone else encountered it (at least not that was reported)
I should have a fix soon. It will go into the 1.4.0 RC soon as well

@yoavniran
Copy link
Collaborator

@mgrunberg v1.4.0-rc.1 is available now with the fix.
Would be great if you can give it a try and let me know it solves your issue

@mgrunberg
Copy link
Author

@yoavniran tested and v1.4.0-rc1 solves my issue! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants