fix(fetch): drop broken fieldsFirst flag

ardatan · Oct 23, 2022 · ab5fb52 · ab5fb52
1 parent 5351291
commit ab5fb52
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 166 deletions.
diff --git a/.changeset/shiny-apples-travel.md b/.changeset/shiny-apples-travel.md
@@ -0,0 +1,5 @@
+---
+'@whatwg-node/fetch': minor
+---
+
+Drop broken `fieldsFirst` flag
diff --git a/packages/fetch/dist/getFormDataMethod.js b/packages/fetch/dist/getFormDataMethod.js
@@ -3,31 +3,6 @@ const { resolve } = require('path');
 const streams = require("stream");
 
 module.exports = function getFormDataMethod(File, limits) {
-  function consumeStreamAsFile({
-    name,
-    filename,
-    mimeType,
-    fileStream,
-    formData,
-  }) {
-    return new Promise((resolve, reject) => {
-      const chunks = [];
-      fileStream.on('limit', () => {
-        reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`));
-      })
-      fileStream.on('data', (chunk) => {
-        chunks.push(...chunk);
-      })
-      fileStream.on('close', () => {
-        if (fileStream.truncated) {
-          reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`));
-        }
-        const file = new File([new Uint8Array(chunks)], filename, { type: mimeType });
-        formData.set(name, file);
-        resolve(file);
-      });
-    })
-  }
 
   return function formData() {
     if (this.body == null) {
@@ -57,56 +32,20 @@ module.exports = function getFormDataMethod(File, limits) {
         reject(new Error(`Fields limit exceeded: ${limits.fields}`));
       })
       bb.on('file', (name, fileStream, { filename, mimeType }) => {
-        let file$;
-        if (limits && limits.fieldsFirst) {
-          resolve(formData);
-          const fakeFileObj = {
-            name: filename,
-            type: mimeType,
-          }
-          Object.setPrototypeOf(fakeFileObj, File.prototype);
-          formData.set(name, new Proxy(fakeFileObj, {
-            get: (target, prop) => {
-              switch(prop) {
-                case 'name':
-                  return filename;
-                case 'type':
-                  return mimeType;
-                case 'stream':
-                  return () => fileStream;
-                case 'size':
-                  throw new Error(`Cannot access file size before consuming the stream.`);
-                case 'slice':
-                  throw new Error(`Cannot slice file before consuming the stream.`);
-                case 'text':
-                case 'arrayBuffer':
-                  return () => {
-                    if (!file$) {
-                      file$ = consumeStreamAsFile({
-                        name,
-                        filename,
-                        mimeType,
-                        fileStream,
-                        formData,
-                      })
-                    }
-                    return file$.then(file => file[prop]());
-                  }
-              }
-            },
-          }))
-        } else {
-          if (!file$) {
-            file$ = consumeStreamAsFile({
-              name,
-              filename,
-              mimeType,
-              fileStream,
-              formData,
-            })
+        const chunks = [];
+        fileStream.on('limit', () => {
+          reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`));
+        })
+        fileStream.on('data', (chunk) => {
+          chunks.push(Buffer.from(chunk));
+        })
+        fileStream.on('close', () => {
+          if (fileStream.truncated) {
+            reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`));
           }
-          file$.catch(reject);
-        }
+          const file = new File(chunks, filename, { type: mimeType });
+          formData.set(name, file);
+        });
       })
       bb.on('filesLimit', () => {
         reject(new Error(`Files limit exceeded: ${limits.files}`));

diff --git a/packages/fetch/dist/index.d.ts b/packages/fetch/dist/index.d.ts
@@ -47,8 +47,6 @@ declare module "@whatwg-node/fetch" {
     parts?: number;
     /* For multipart forms, the max number of header key-value pairs to parse. Default: 2000. */
     headerSize?: number;
-    /* For multipart forms, enable this if your data has fields first, then files. Default: false. */
-    fieldsFirst?: boolean;
   }
   export const createFetch: (opts?: { useNodeFetch?: boolean; formDataLimits?: FormDataLimits }) => ({
     fetch: typeof _fetch,

diff --git a/packages/fetch/tests/getFormDataMethod.spec.ts b/packages/fetch/tests/getFormDataMethod.spec.ts
@@ -1,47 +1,38 @@
 import { createTestContainer } from '../../server/test/create-test-container';
 
 describe('getFormDataMethod', () => {
-  ['fieldsFirst:true', 'fieldsFirst:false'].forEach(fieldsFirstFlag => {
-    describe(fieldsFirstFlag, () => {
-      createTestContainer(
-        fetchAPI => {
-          it('should parse fields correctly', async () => {
-            const formData = new fetchAPI.FormData();
-            formData.append('greetings', 'Hello world!');
-            formData.append('bye', 'Goodbye world!');
-            const request = new fetchAPI.Request('http://localhost:8080', {
-              method: 'POST',
-              body: formData,
-            });
-            const formdata = await request.formData();
-            expect(formdata.get('greetings')).toBe('Hello world!');
-            expect(formdata.get('bye')).toBe('Goodbye world!');
-          });
-          it('should parse and receive text files correctly', async () => {
-            const formData = new fetchAPI.FormData();
-            const greetingsFile = new fetchAPI.File(['Hello world!'], 'greetings.txt', { type: 'text/plain' });
-            const byeFile = new fetchAPI.File(['Goodbye world!'], 'bye.txt', { type: 'text/plain' });
-            formData.append('greetings', greetingsFile);
-            formData.append('bye', byeFile);
-            const request = new fetchAPI.Request('http://localhost:8080', {
-              method: 'POST',
-              body: formData,
-            });
-            const formdata = await request.formData();
-            const receivedGreetingsFile = formdata.get('greetings') as File;
-            const receivedGreetingsText = await receivedGreetingsFile.text();
-            expect(receivedGreetingsText).toBe('Hello world!');
-            const receivedByeFile = formdata.get('bye') as File;
-            const receivedByeText = await receivedByeFile.text();
-            expect(receivedByeText).toBe('Goodbye world!');
-          });
-        },
-        {
-          formDataLimits: {
-            fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true',
-          },
-        }
-      );
-    });
-  });
+  createTestContainer(
+    fetchAPI => {
+      it('should parse fields correctly', async () => {
+        const formData = new fetchAPI.FormData();
+        formData.append('greetings', 'Hello world!');
+        formData.append('bye', 'Goodbye world!');
+        const request = new fetchAPI.Request('http://localhost:8080', {
+          method: 'POST',
+          body: formData,
+        });
+        const formdata = await request.formData();
+        expect(formdata.get('greetings')).toBe('Hello world!');
+        expect(formdata.get('bye')).toBe('Goodbye world!');
+      });
+      it('should parse and receive text files correctly', async () => {
+        const formData = new fetchAPI.FormData();
+        const greetingsFile = new fetchAPI.File(['Hello world!'], 'greetings.txt', { type: 'text/plain' });
+        const byeFile = new fetchAPI.File(['Goodbye world!'], 'bye.txt', { type: 'text/plain' });
+        formData.append('greetings', greetingsFile);
+        formData.append('bye', byeFile);
+        const request = new fetchAPI.Request('http://localhost:8080', {
+          method: 'POST',
+          body: formData,
+        });
+        const formdata = await request.formData();
+        const receivedGreetingsFile = formdata.get('greetings') as File;
+        const receivedGreetingsText = await receivedGreetingsFile.text();
+        expect(receivedGreetingsText).toBe('Hello world!');
+        const receivedByeFile = formdata.get('bye') as File;
+        const receivedByeText = await receivedByeFile.text();
+        expect(receivedByeText).toBe('Goodbye world!');
+      });
+    }
+  );
 });
diff --git a/packages/server/test/formdata.spec.ts b/packages/server/test/formdata.spec.ts
@@ -10,54 +10,45 @@ describe('FormData', () => {
   afterAll(done => {
     testServer.server.close(done);
   });
-  ['fieldsFirst:true', 'fieldsFirst:false'].forEach(fieldsFirstFlag => {
-    createTestContainer(
-      fetchAPI => {
-        describe(fieldsFirstFlag, () => {
-          it('should forward formdata correctly', async () => {
-            let receivedFieldContent: string | undefined;
-            let receivedFileName: string | undefined;
-            let receivedFileType: string | undefined;
-            let receivedFileContent: string | undefined;
-            const adapter = createServerAdapter(async request => {
-              try {
-                const body = await request.formData();
-                receivedFieldContent = body.get('foo') as string;
-                const file = body.get('baz') as File;
-                receivedFileName = file.name;
-                receivedFileType = file.type;
-                receivedFileContent = await file.text();
-              } catch (e: any) {
-                return new fetchAPI.Response(e.stack, {
-                  status: 500,
-                });
-              }
-              return new fetchAPI.Response(null, {
-                status: 204,
-              });
-            }, fetchAPI.Request);
-            testServer.server.once('request', adapter);
-            const formData = new fetchAPI.FormData();
-            formData.append('foo', 'bar');
-            formData.append('baz', new fetchAPI.File(['baz'], 'baz.txt', { type: 'text/plain' }));
-            const response = await fetchAPI.fetch(testServer.url, {
-              method: 'POST',
-              body: formData,
+  createTestContainer(
+    fetchAPI => {
+      it('should forward formdata correctly', async () => {
+        let receivedFieldContent: string | undefined;
+        let receivedFileName: string | undefined;
+        let receivedFileType: string | undefined;
+        let receivedFileContent: string | undefined;
+        const adapter = createServerAdapter(async request => {
+          try {
+            const body = await request.formData();
+            receivedFieldContent = body.get('foo') as string;
+            const file = body.get('baz') as File;
+            receivedFileName = file.name;
+            receivedFileType = file.type;
+            receivedFileContent = await file.text();
+          } catch (e: any) {
+            return new fetchAPI.Response(e.stack, {
+              status: 500,
             });
-            expect(await response.text()).toBe('');
-            expect(response.status).toBe(204);
-            expect(receivedFieldContent).toBe('bar');
-            expect(receivedFileName).toBe('baz.txt');
-            expect(receivedFileType).toBe('text/plain');
-            expect(receivedFileContent).toBe('baz');
+          }
+          return new fetchAPI.Response(null, {
+            status: 204,
           });
+        }, fetchAPI.Request);
+        testServer.server.once('request', adapter);
+        const formData = new fetchAPI.FormData();
+        formData.append('foo', 'bar');
+        formData.append('baz', new fetchAPI.File(['baz'], 'baz.txt', { type: 'text/plain' }));
+        const response = await fetchAPI.fetch(testServer.url, {
+          method: 'POST',
+          body: formData,
         });
-      },
-      {
-        formDataLimits: {
-          fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true',
-        },
-      }
-    );
-  });
+        expect(await response.text()).toBe('');
+        expect(response.status).toBe(204);
+        expect(receivedFieldContent).toBe('bar');
+        expect(receivedFileName).toBe('baz.txt');
+        expect(receivedFileType).toBe('text/plain');
+        expect(receivedFileContent).toBe('baz');
+      });
+    }
+  );
 });