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

Fixed no response return onload callback #631

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Yahasana
Copy link

If there is nothing return from onload callback, use xhr.response by default

If there is nothing return from `onload` callback, use xhr.response by default
@rikschennink
Copy link
Collaborator

There is no related issue. This is also not a bug. Please in future first submit issue with bug report then we can discuss a PR.

@Yahasana
Copy link
Author

Yahasana commented Dec 29, 2020

It's not a bug, but it's a bad implement. It's take me a whole day to investigate what the hell of failing revert upload. e.g.

FilePond.setOptions({
    server: {
        process: {
            method: "POST",
            onload: (response) => {
                console.log(response)
                // return response  <-- revert upload by serverId rely on this return! wtf
            }
    }
})

@rikschennink
Copy link
Collaborator

I'm not comfortable changing this behaviour at this point in time. If you set onload you need to return a value.

This will go wrong when dev returns 0 or false or another falsy value in the onload hook.

onload(xhr.response) || xhr.response,

@rikschennink
Copy link
Collaborator

Will re-open, PR should be updated to fix above issue with falsy values.

@rikschennink rikschennink reopened this Jan 4, 2021
@Yahasana
Copy link
Author

Yahasana commented Jan 5, 2021

why people returns 0 or false as keyId? bad practice it's

@rikschennink
Copy link
Collaborator

@Yahasana file server id could potentially 0

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