STRF-4803 - adding helper function to remove empty files from jquery … #1210
Conversation
|
Autotagging @bigcommerce/storefront-team @davidchin |
| for (const pair of form.entries()) { | ||
| const key = pair[0]; | ||
| const val = pair[1]; | ||
| if (typeof val === 'object' && val instanceof File) { |
There was a problem hiding this comment.
🍹 Are there any instances where val instanceof File passes but typeof val === 'object' does not? You can simplify this to just an instanceof check if not.
There was a problem hiding this comment.
guess not - was trying to be safe for the undef case but realized that "undefined instanceof File" is safe
There was a problem hiding this comment.
would it be better actually to leave the object check and just check if the size/name fields are propertyOf -> and then check the values are not empty/undefined?
tpietsch
force-pushed
the
STRF-4803
branch
3 times, most recently
from
e6e86ac
to
c90e4df
Compare
| */ | ||
| filterEmptyFilesFromForm(form) { | ||
| try { | ||
| for (const pair of form.entries()) { |
There was a problem hiding this comment.
🍹 could use destructuring (i.e. const [key, val] of form.entries())
There was a problem hiding this comment.
do we have browser support requirements?
There was a problem hiding this comment.
we do but don't need to worry about them in this case. we've got webpack and babel to transpile down to the envs we specify in the browserlist file. :)
| */ | ||
| filterEmptyFilesFromForm(form) { | ||
| try { | ||
| for (const [key, val] of form.entries()) { |
There was a problem hiding this comment.
looks like you should be able to use const [key, val] of form
An object implementing FormData can directly be used in a for...of structure, instead of entries(): for (var p of myFormData) is equivalent to for (var p of myFormData.entries()). (https://developer.mozilla.org/en-US/docs/Web/API/FormData). maybe rename the param to formData to be explicit?
| filterEmptyFilesFromForm(form) { | ||
| try { | ||
| for (const [key, val] of form.entries()) { | ||
| if (val instanceof File) { |
There was a problem hiding this comment.
might as well pull the second conditional into the first:
if (val instanceof File && val.name === '' && val.size === 0) {
could also simplify to if (val instanceof File and !val.name && !val.size) {
https://stackoverflow.com/questions/49672992/ajax-request-fails-when-sending-formdata-including-empty-file-input-in-safari
our version of jquery has a problem making ajax calls with empty files in safari. Appears the solution is to remove empty files before sending unless anyone has other suggestions.
@bigcommerce/storefront-team @bigcommerce/platform-engineering