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

fix: buildRequest of multipart/formdata with array entries #1527

Merged
merged 1 commit into from
May 13, 2020

Conversation

tim-lai
Copy link
Contributor

@tim-lai tim-lai commented May 5, 2020

Description

Multipart/form-data support for

  • OAS 2.0
  • OAS 3.0
  • includes file and files

Also fixes: cross browser and node support

  • @timlai/isomorphic-form-data single replacement for isomorphic-form-data, form-data, and formdata-node. However, under the hood, @timlai/isomorphic-form-data uses formdata-node, which replaces form-data, to get FormData methods parity with browser

housekeeping: update webpack config to support webpack4

  • use rules instead of loaders

Motivation and Context

Fixes #1526
Fixes #1505
Fixes #1480
Fixes #1470
supersedes #1485

How Has This Been Tested?

New tests created in test/http-multipart.js

  • includes skipped test to live local node/express server

Tested on Node v13.9

Screenshots (if appropriate):

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tim-lai tim-lai requested review from char0n and webron May 5, 2020 18:52
@tim-lai tim-lai self-assigned this May 5, 2020
@tim-lai
Copy link
Contributor Author

tim-lai commented May 5, 2020

please build

1 similar comment
@tim-lai
Copy link
Contributor Author

tim-lai commented May 5, 2020

please build

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done on fix! Went through the PR, left some code review comments

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
webpack.common.js Show resolved Hide resolved
webpack/_config-builder.js Outdated Show resolved Hide resolved
test/http-multipart.js Outdated Show resolved Hide resolved
test/http-multipart.js Outdated Show resolved Hide resolved
test/http-multipart.js Outdated Show resolved Hide resolved
src/execute/oas3/build-request.js Outdated Show resolved Hide resolved
src/http.js Show resolved Hide resolved
@@ -1538,6 +1538,97 @@
"@octokit/types": "^2.0.0"
Copy link
Member

@char0n char0n May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did we regenerated this file? I see a lot of changes that looks like accidental. When we run following command against master:

$ npm uninstall --save isomorphic-form-data --verbose
$ npm i --save @tim-lai/isomorphic-form-data --verbose

and then run diff,

$ git diff

I can see only the following changes:

diff --git a/package-lock.json b/package-lock.json
index 3a8ce24..cfa4576 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1701,6 +1701,14 @@
         "defer-to-connect": "^1.0.1"
       }
     },
+    "@tim-lai/isomorphic-form-data": {
+      "version": "1.0.0",
+      "resolved": "https://registry.npmjs.org/@tim-lai/isomorphic-form-data/-/isomorphic-form-data-1.0.0.tgz",
+      "integrity": "sha512-ukWANS+8w3NCj4Qmnc4zo0XWcoqnDAiqaBXVdh/z6CPtOL+z6I13H3AKOdGzeAaPCeXRL3sPqaPgrSqEgtF8Xg==",
+      "requires": {
+        "formdata-node": "^2.1.1"
+      }
+    },
     "@types/babel__core": {
       "version": "7.1.7",
       "resolved": "https://registry.npmjs.org/@types/babel__core/-/babel__core-7.1.7.tgz",
@@ -2533,14 +2541,6 @@
       "integrity": "sha512-+Ryf6g3BKoRc7jfp7ad8tM4TtMiaWvbF/1/sQcZPkkS7ag3D5nMBCe2UfOTONtAkaG0tO0ij3C5Lwmf1EiyjHg==",
       "dev": true
     },
-    "async": {
-      "version": "2.6.3",
-      "resolved": "https://registry.npmjs.org/async/-/async-2.6.3.tgz",
-      "integrity": "sha512-zflvls11DCy+dQWzTW2dzuilv8Z5X/pjfmZOWba6TNIVDm+2UDaJmXSOXlasHKfNBs8oo3M0aT50fDEWfKZjXg==",
-      "requires": {
-        "lodash": "^4.17.14"
-      }
-    },
     "async-each": {
       "version": "1.0.3",
       "resolved": "https://registry.npmjs.org/async-each/-/async-each-1.0.3.tgz",
@@ -3759,6 +3759,7 @@
       "version": "1.0.8",
       "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz",
       "integrity": "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==",
+      "dev": true,
       "requires": {
         "delayed-stream": "~1.0.0"
       }
@@ -4525,7 +4526,8 @@
     "delayed-stream": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz",
-      "integrity": "sha1-3zrhmayt+31ECqrgsp4icrJOxhk="
+      "integrity": "sha1-3zrhmayt+31ECqrgsp4icrJOxhk=",
+      "dev": true
     },
     "delegates": {
       "version": "1.0.0",
@@ -6211,14 +6213,24 @@
       "integrity": "sha1-+8cfDEGt6zf5bFd60e1C2P2sypE=",
       "dev": true
     },
-    "form-data": {
-      "version": "1.0.1",
-      "resolved": "https://registry.npmjs.org/form-data/-/form-data-1.0.1.tgz",
-      "integrity": "sha1-rjFduaSQf6BlUCMEpm13M0de43w=",
+    "formdata-node": {
+      "version": "2.1.1",
+      "resolved": "https://registry.npmjs.org/formdata-node/-/formdata-node-2.1.1.tgz",
+      "integrity": "sha512-XfwCd3Edkt2N81Uh1G72kakbavjEOhRuGdiOV6Ekba/DrFrWFWgkjN4dZn5EPNbjmg+QC+i0ckeYukAbuTRHjw==",
       "requires": {
-        "async": "^2.0.1",
-        "combined-stream": "^1.0.5",
-        "mime-types": "^2.1.11"
+        "@babel/runtime": "7.8.4",
+        "mime-types": "2.1.26",
+        "nanoid": "2.1.11"
+      },
+      "dependencies": {
+        "@babel/runtime": {
+          "version": "7.8.4",
+          "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.8.4.tgz",
+          "integrity": "sha512-neAp3zt80trRVBI1x0azq6c57aNBqYZH8KhMm3TaB7wEI5Q4A2SHfBHE8w9gOhI/lrqxtEbXZgQIrHP+wvSGwQ==",
+          "requires": {
+            "regenerator-runtime": "^0.13.2"
+          }
+        }
       }
     },
     "fragment-cache": {
@@ -8244,14 +8256,6 @@
       "integrity": "sha512-S/2fF5wH8SJA/kmwr6HYhK/RI/OkhD84k8ntalo0iJjZikgq1XFvR5M8NPT1x5F7fBwCG3qHfnzeP/Vh/ZxCUA==",
       "dev": true
     },
-    "isomorphic-form-data": {
-      "version": "0.0.1",
-      "resolved": "https://registry.npmjs.org/isomorphic-form-data/-/isomorphic-form-data-0.0.1.tgz",
-      "integrity": "sha1-Am9ifgMrDNhBPsyHVZKLlKRosGI=",
-      "requires": {
-        "form-data": "^1.0.0-rc3"
-      }
-    },
     "isstream": {
       "version": "0.1.2",
       "resolved": "https://registry.npmjs.org/isstream/-/isstream-0.1.2.tgz",
@@ -10337,6 +10341,11 @@
       "integrity": "sha512-INOFj37C7k3AfaNTtX8RhsTw7qRy7eLET14cROi9+5HAVbbHuIWUHEauBv5qT4Av2tWasiTY1Jw6puUNqRJXQg==",
       "dev": true
     },
+    "nanoid": {
+      "version": "2.1.11",
+      "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-2.1.11.tgz",
+      "integrity": "sha512-s/snB+WGm6uwi0WjsZdaVcuf3KJXlfGl2LcxgwkEwJF0D/BWzVWAZW/XY4bFaiR7s0Jk3FPvlnepg1H1b1UwlA=="
+    },
     "nanomatch": {
       "version": "1.2.13",
       "resolved": "https://registry.npmjs.org/nanomatch/-/nanomatch-1.2.13.tgz",
diff --git a/package.json b/package.json
index 5bb4d9f..5cf3581 100644
--- a/package.json
+++ b/package.json
@@ -98,6 +98,7 @@
   "dependencies": {
     "@babel/runtime-corejs2": "^7.0.0",
     "@kyleshockey/object-assign-deep": "^0.4.0",
+    "@tim-lai/isomorphic-form-data": "^1.0.0",
     "btoa": "1.1.2",
     "buffer": "^5.1.0",
     "cookie": "^0.3.1",
@@ -105,7 +106,6 @@
     "deep-extend": "^0.5.1",
     "encode-3986": "^1.0.0",
     "fast-json-patch": "~2.1.0",
-    "isomorphic-form-data": "0.0.1",
     "js-yaml": "^3.13.1",
     "lodash": "^4.17.14",
     "qs": "^6.3.0",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. not seeing what you are seeing. I npm i again, and package-lock is current with most recent update.

form-data and isomorphic-form-data are valid removal. formdata-node is valid addition (from @tim-lai/isomorphic-form-data). nanoid is dependency from formdata-node

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess your npm i have updated the dep tree then? Let's research this. Accidental updates of dep tree should not happen unless we explicitly want to.

@tim-lai tim-lai force-pushed the bug/1526-multipart branch 3 times, most recently from 28590e3 to 81f1573 Compare May 13, 2020 22:43
- OAS 2.0
- OAS 3.0

fix: cross browser and node support
- @timlai/isomorphic-form-data replaces isomorphic-form-data and form-data

housekeeping: update webpack config to support webpack4
- use rules instead of loaders
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants