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

Changing enhancer during Render causes upload to stop #803

Closed
raybooysen opened this issue Dec 23, 2024 · 7 comments
Closed

Changing enhancer during Render causes upload to stop #803

raybooysen opened this issue Dec 23, 2024 · 7 comments

Comments

@raybooysen
Copy link
Contributor

raybooysen commented Dec 23, 2024

Describe the bug
The setup is simple, u ploading multiple files to a back end. In my usage, I create an <Uploady> component with an TusSender enhancement. As part of that sender I create it as such:

return (
        <Uploady
            concurrent={true}
            maxConcurrent={8}
            destination={{
                url: url,
                method: 'POST',
                headers: {
                    'X-custom-header': customValueThatMayChangeDuringRuntime,
                },
            }}
            enhancer={TusSender({
                resume: true,
                chunkSize: 1024 * 1024,
                resumeHeaders: {
                    'X-custom-header': customValueThatMayChangeDuringRuntime,
                },
            })}
        >

In this case the TusSender is created on each render. When I select multiple files to upload, all files are uploaded to the back-end. However, the useItemProgressListener and useItemFinishListener hook callbacks are only called for the first file. No callbacks are called for the 2nd and subsequent files. This means I cannot track progress of the file uploads.

I've managed to create a work-around using useMemo for the enhancer, which suggests the introduction of a new instance of the TusSender on each render causes some internal Uploady state to become invalid.

const enhancer = useMemo(() => {
        return TusSender({
            resume: true,
            chunkSize: 1024 * 1024,
            resumeHeaders: {
                'X-custom-header': customValueThatMayChangeDuringRuntime,
            },
        });
    }, [customValueThatMayChangeDuringRuntime]);

    return (
        <Uploady
            concurrent={true}
            maxConcurrent={8}
            destination={{
                url: url,
                method: 'POST',
                headers: {
                    'X-custom-header': customValueThatMayChangeDuringRuntime,
                },
            }}
            enhancer={enhancer}
        >

To Reproduce
Steps to reproduce the behavior:

  1. Create a simple Uploady use case
  2. Set the TusSender as an enhancer as per the code above
  3. Upload multiple files
  4. Note the lack of callbacks in the two hooks

Expected behavior
The hooks useItemProgressListener and useItemFinishListener should report status for the subsequent files

Versions
1.9.0
Chrome

@yoavniran
Copy link
Collaborator

@raybooysen it is indeed inadvisable to re-create an enhancer at render time of the component that renders Uploady.
Not just related to Tus, in general, enhancers should be created outside the rendering flow and be used as singletons or at least memoized.

Can you explain why you're creating your own Tus Enhancer and not using TusUploady? Is there a specific reason?

If not, I strongly recommend you do that, it should also help with the issue at hand.

@raybooysen
Copy link
Contributor Author

In this case, the as per the example, the custom header has a value that changes during runtime. Imagine a fingerprint, a token, something like that. To accomplish this I need to be able to modify the headers, although there are other usecases like changing the URL.

Since the enhancer is used within React within JSX (or TSX), the expectation would be that the internal state would be idempotent to the enhancer changing, OR that the TusSender memoises based on it's own props.

@yoavniran
Copy link
Collaborator

There are built-in ways to support your use-case.

The Destination details can be changed/updated in ways that dont require re-rendering the (Tus-)Uploady provider.

See these guides for more information:

Also, from the Context, you can use the upload method that accepts UploadOptions where things like headers can be overridden, in case you want more programmatic control when adding files to be uploaded.

@raybooysen
Copy link
Contributor Author

Using the useRequestPreSend hook, I can add headers etc., however there is a HEAD request that goes out where those headers are not applied.

Removing the TusSender, then this HEAD request is not made.

@raybooysen
Copy link
Contributor Author

I browsed the code this morning: https://github.com/rpldy/react-uploady/tree/master/packages/core/tus-sender, allows you to set the headers as we're doing now, and in resumeUpload.js of tusSender, when it creates the HEAD request, but I could not find where those resumeHeaders are merged with the options returned in the useRequestPreSend hook.

Am I missing something obvious?

To be clear, the useRequestPreSend hook works for requests, but the headers for the HEAD call that tusSender fires are not modified to include my dynamic data.

@yoavniran
Copy link
Collaborator

Thanks for clarifying, I didn't notice you needed the HEAD call headers.
In this case, useTusResumeStartListener should do the trick.

Merging happens on line 127 in the file you mentioned.

@raybooysen
Copy link
Contributor Author

Works a charm, thank you

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