-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix For When user selects Empty files using File Upload #3051
Conversation
reflex/.templates/web/utils/state.js
Outdated
|
||
if (event.payload.files === undefined || event.payload.files.length === 0){ | ||
// Submit the event over the websocket to trigger the event handler. | ||
return await applyEvent(Event(event.name, event.payload.files), socket) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the payload is supposed to be a object, not an array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean return await applyEvent(Event(event.name, event.payload), socket)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem is that event.payload
contains keys that do not map to an arbitrary event handler on the backend.
for example, maybe it works if the backend event handler has a files
arg; but if that arg is named fools
, then we get an error on the backend about unknown keyword argument.
reflex/.templates/web/utils/state.js
Outdated
@@ -381,7 +387,7 @@ export const uploadFiles = async ( | |||
) => { | |||
// return if there's no file to upload | |||
if (files === undefined || files.length === 0) { | |||
return false; | |||
files = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this change doing? if we end up hitting this code, won't it ultimately throw that "Did not find CR at end of boundary (40)" error in the upload handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, resolved it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only thing that still makes me nervous about this is that, if the handler doesn't specify a default for the list[rx.UploadFile]
arg, then there will be a traceback printed on the backend, whereas before, this was just a no op.
i think that's okay though. it will still be a no-op, and if they want to avoid the traceback, they can add a default value...
i think we can put it in a doc somewhere ? or leave it like that? what dyu suggest ? |
All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features:
After these steps, you're ready to open a pull request.