Skip to content

Commit

Permalink
Fix submitting form data when file input is empty (#12)
Browse files Browse the repository at this point in the history
Co-authored-by: Jacob Ebey <jacob.ebey@live.com>
  • Loading branch information
cwohlman and jacob-ebey committed Jul 14, 2022
1 parent dcfcac4 commit 6521895
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 68 deletions.
6 changes: 6 additions & 0 deletions .changeset/lemon-nails-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@remix-run/web-form-data": patch
---

Fix submitting form data when file input is empty. Addresses https://github.com/remix-run/remix/pull/3576

122 changes: 61 additions & 61 deletions packages/form-data/src/form-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@ export class FormData {
)
: new TypeError(
"FormData constructor: Argument 1 does not implement interface HTMLFormElement."
)
);

throw error
throw error;
}

/**
* @private
* @readonly
* @type {Array<[string, FormDataEntryValue]>}
*/
this._entries = []
this._entries = [];

Object.defineProperty(this, "_entries", { enumerable: false })
Object.defineProperty(this, "_entries", { enumerable: false });
}
get [Symbol.toStringTag]() {
return "FormData"
return "FormData";
}

/**
Expand All @@ -54,7 +54,7 @@ export class FormData {
),
filename
) {
this._entries.push([name, toEntryValue(value, filename)])
this._entries.push([name, toEntryValue(value, filename)]);
}

/**
Expand All @@ -65,16 +65,16 @@ export class FormData {
delete(
name = panic(new TypeError("FormData.delete: requires string argument"))
) {
const entries = this._entries
let index = 0
const entries = this._entries;
let index = 0;
while (index < entries.length) {
const [entryName] = /** @type {[string, FormDataEntryValue]}*/ (
entries[index]
)
);
if (entryName === name) {
entries.splice(index, 1)
entries.splice(index, 1);
} else {
index++
index++;
}
}
}
Expand All @@ -90,10 +90,10 @@ export class FormData {
get(name = panic(new TypeError("FormData.get: requires string argument"))) {
for (const [entryName, value] of this._entries) {
if (entryName === name) {
return value
return value;
}
}
return null
return null;
}

/**
Expand All @@ -106,13 +106,13 @@ export class FormData {
getAll(
name = panic(new TypeError("FormData.getAll: requires string argument"))
) {
const values = []
const values = [];
for (const [entryName, value] of this._entries) {
if (entryName === name) {
values.push(value)
values.push(value);
}
}
return values
return values;
}

/**
Expand All @@ -124,10 +124,10 @@ export class FormData {
has(name = panic(new TypeError("FormData.has: requires string argument"))) {
for (const [entryName] of this._entries) {
if (entryName === name) {
return true
return true;
}
}
return false
return false;
}

/**
Expand All @@ -144,27 +144,27 @@ export class FormData {
value = panic(new TypeError("FormData.set: requires at least 2 arguments")),
filename
) {
let index = 0
const { _entries: entries } = this
const entryValue = toEntryValue(value, filename)
let wasSet = false
let index = 0;
const { _entries: entries } = this;
const entryValue = toEntryValue(value, filename);
let wasSet = false;
while (index < entries.length) {
const entry = /** @type {[string, FormDataEntryValue]}*/ (entries[index])
const entry = /** @type {[string, FormDataEntryValue]}*/ (entries[index]);
if (entry[0] === name) {
if (wasSet) {
entries.splice(index, 1)
entries.splice(index, 1);
} else {
wasSet = true
entry[1] = entryValue
index++
wasSet = true;
entry[1] = entryValue;
index++;
}
} else {
index++
index++;
}
}

if (!wasSet) {
entries.push([name, entryValue])
entries.push([name, entryValue]);
}
}

Expand All @@ -173,7 +173,7 @@ export class FormData {
* contained in this object.
*/
entries() {
return this._entries.values()
return this._entries.values();
}

/**
Expand All @@ -184,7 +184,7 @@ export class FormData {
*/
*keys() {
for (const [name] of this._entries) {
yield name
yield name;
}
}

Expand All @@ -196,12 +196,12 @@ export class FormData {
*/
*values() {
for (const [_, value] of this._entries) {
yield value
yield value;
}
}

[Symbol.iterator]() {
return this._entries.values()
return this._entries.values();
}

/**
Expand All @@ -211,7 +211,7 @@ export class FormData {
*/
forEach(fn, thisArg) {
for (const [key, value] of this._entries) {
fn.call(thisArg, value, key, this)
fn.call(thisArg, value, key, this);
}
}
}
Expand All @@ -220,8 +220,8 @@ export class FormData {
* @param {any} value
* @returns {value is HTMLFormElement}
*/
const isHTMLFormElement = value =>
Object.prototype.toString.call(value) === "[object HTMLFormElement]"
const isHTMLFormElement = (value) =>
Object.prototype.toString.call(value) === "[object HTMLFormElement]";

/**
* @param {string|Blob|File} value
Expand All @@ -230,33 +230,33 @@ const isHTMLFormElement = value =>
*/
const toEntryValue = (value, filename) => {
if (isFile(value)) {
return filename != null ? new BlobFile([value], filename, value) : value
return filename != null ? new BlobFile([value], filename, value) : value;
} else if (isBlob(value)) {
return new BlobFile([value], filename != null ? filename : "blob")
return new BlobFile([value], filename != null ? filename : "blob");
} else {
if (filename != null) {
if (filename != null && filename != "") {
throw new TypeError(
"filename is only supported when value is Blob or File"
)
);
}
return `${value}`
return `${value}`;
}
}
};

/**
* @param {any} value
* @returns {value is File}
*/
const isFile = value =>
const isFile = (value) =>
Object.prototype.toString.call(value) === "[object File]" &&
typeof value.name === "string"
typeof value.name === "string";

/**
* @param {any} value
* @returns {value is Blob}
*/
const isBlob = value =>
Object.prototype.toString.call(value) === "[object Blob]"
const isBlob = (value) =>
Object.prototype.toString.call(value) === "[object Blob]";

/**
* Simple `File` implementation that just wraps a given blob.
Expand All @@ -269,18 +269,18 @@ const BlobFile = class File {
* @param {FilePropertyBag} [options]
*/
constructor([blob], name, { lastModified = Date.now() } = {}) {
this.blob = blob
this.name = name
this.lastModified = lastModified
this.blob = blob;
this.name = name;
this.lastModified = lastModified;
}
get webkitRelativePath() {
return ""
return "";
}
get size() {
return this.blob.size
return this.blob.size;
}
get type() {
return this.blob.type
return this.blob.type;
}
/**
*
Expand All @@ -289,26 +289,26 @@ const BlobFile = class File {
* @param {string} [contentType]
*/
slice(start, end, contentType) {
return this.blob.slice(start, end, contentType)
return this.blob.slice(start, end, contentType);
}
stream() {
return this.blob.stream()
return this.blob.stream();
}
text() {
return this.blob.text()
return this.blob.text();
}
arrayBuffer() {
return this.blob.arrayBuffer()
return this.blob.arrayBuffer();
}
get [Symbol.toStringTag]() {
return "File"
return "File";
}
}
};

/**
* @param {*} error
* @returns {never}
*/
const panic = error => {
throw error
}
const panic = (error) => {
throw error;
};
25 changes: 18 additions & 7 deletions packages/form-data/test/form-data.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { assert } from "./test.js";
/**
* @param {import('./test').Test} test
*/
export const test = test => {
export const test = (test) => {
test("test baisc", async () => {
assert.equal(typeof FormData, "function");
assert.isEqual(typeof lib.FormData, "function");
Expand Down Expand Up @@ -89,6 +89,17 @@ export const test = test => {
assert.equal(file2.lastModified, 123, "lastModified should be 123");
});

// This mimics the payload sent by a browser when a file input
// exists but is not filled out.
test("filename on string contents", () => {
const formData = new FormData();
formData.set("file-3", new Blob([]), "");
const file3 = /** @type {File} */ (formData.get("file-3"));
assert.equal(file3.constructor.name, "File");
assert.equal(file3.name, "");
assert.equal(file3.type, "");
});

test("throws on few args", () => {
const data = new FormData();
// @ts-expect-error
Expand Down Expand Up @@ -150,7 +161,7 @@ export const test = test => {
["keyA", "val1"],
["keyA", "val2"],
["keyB", "val3"],
["keyA", "val4"]
["keyA", "val4"],
]
);
});
Expand All @@ -167,7 +178,7 @@ export const test = test => {
[...data],
[
["keyA", "val3"],
["keyB", "val3"]
["keyB", "val3"],
]
);
});
Expand All @@ -181,7 +192,7 @@ export const test = test => {
[...data],
[
["keyB", "val3"],
["keyA", "val3"]
["keyA", "val3"],
]
);
});
Expand All @@ -207,21 +218,21 @@ export const test = test => {
assert.deepEqual([...data], [["n2", "v2"]]);
});

test("Shold return correct filename with File", () => {
test("Should return correct filename with File", () => {
const data = new FormData();
data.set("key", new File([], "doc.txt"));
const file = /** @type {File} */ (data.get("key"));
assert.equal("doc.txt", file.name);
});

test("Shold return correct filename with Blob filename", () => {
test("Should return correct filename with Blob filename", () => {
const data = new FormData();
data.append("key", new Blob(), "doc.txt");
const file = /** @type {File} */ (data.get("key"));
assert.equal("doc.txt", file.name);
});

test("Shold return correct filename with just Blob", () => {
test("Should return correct filename with just Blob", () => {
const data = new FormData();
data.append("key", new Blob());
const file = /** @type {File} */ (data.get("key"));
Expand Down

0 comments on commit 6521895

Please sign in to comment.