Skip to content

feat(formdata): use formdata-node to handle form data #2154

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

Merged
merged 10 commits into from
Jul 27, 2021
4 changes: 2 additions & 2 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ jobs:

steps:
- uses: actions/checkout@v2
- name: Use Node.js 8.x
- name: Use Node.js 12.4
uses: actions/setup-node@v2
with:
node-version: 8.x
node-version: 12.x
- name: Cache Node Modules
id: cache-node-modules
uses: actions/cache@v2
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Swagger Client Version | Release Date | OpenAPI Spec compatibility | Notes

### Runtime

- Node.js `>=` 10.x
- Node.js `>=` 12.4.x
- `swagger-client` works in the latest versions of Chrome, Safari, Firefox, and Edge.

## Security contact
Expand Down
6 changes: 2 additions & 4 deletions docs/development/setting-up.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ Current Node.js:
- NPM >=7.10.x

Current Node.js Active LTS:
- Node.js 14.x
- Node.js >=14.x
- NPM >=6.14.x

Current Node.js Maintenance LTS:
- Node.js >=12.x
- Node.js >=12.4
- NPM >=6.12.x

> Note: our build artifacts should also work on older node versions such as node>=8.

### Steps

1. `git clone https://github.com/swagger-api/swagger-js.git`
Expand Down
4 changes: 2 additions & 2 deletions docs/usage/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

### NPM Registry

We publish two modules to npm: [swagger-client](https://www.npmjs.com/package/swagger-client).
`swagger-client` is meant for consumption by any JavaScript engine (node.js, web, etc...).
We publish single module to npm: [swagger-client](https://www.npmjs.com/package/swagger-client).
`swagger-client` is meant for consumption by any JavaScript engine (node.js, browser, etc...).
The npm package contains transpiled and minified ES5 compatible code.

```shell script
Expand Down
56 changes: 38 additions & 18 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"name": "swagger-client",
"version": "3.13.7",
"description": "SwaggerJS - a collection of interfaces for OAI specs",
"browser": {
"./src/http/fold-formdata-to-request.node.js": "./src/http/fold-formdata-to-request.browser.js"
},
"main": "lib/commonjs.js",
"module": "es/index.js",
"jsnext:main": "es/index.js",
Expand Down Expand Up @@ -41,7 +44,7 @@
"test:unit:coverage": "cross-env BABEL_ENV=commonjs jest --runInBand --config ./config/jest/jest.unit.coverage.config.js",
"test:unit:watch": "cross-env BABEL_ENV=commonjs jest --runInBand --watch --config ./config/jest/jest.unit.config.js",
"test:artifact": "run-s test:artifact:umd:browser test:artifact:es test:artifact:commonjs",
"test:artifact:umd:browser": "npm run build:umd:browser && cross-env BABEL_ENV=commonjs jest --config ./config/jest/jest.artifact-umd-browser.config.js",
"test:artifact:umd:browser": "npm run build:umd:browser && cross-env BABEL_ENV=browser jest --config ./config/jest/jest.artifact-umd-browser.config.js",
"test:artifact:es": "npm run build:es && cross-env BABEL_ENV=commonjs jest --config ./config/jest/jest.artifact-es.config.js",
"test:artifact:commonjs": "npm run build:commonjs && cross-env BABEL_ENV=commonjs jest --config ./config/jest/jest.artifact-commonjs.config.js",
"deps:license": "license-checker --production --csv --out $npm_package_config_deps_check_dir/licenses.csv && license-checker --development --csv --out $npm_package_config_deps_check_dir/licenses-dev.csv",
Expand Down Expand Up @@ -109,7 +112,8 @@
"cross-fetch": "^3.1.4",
"deep-extend": "~0.6.0",
"fast-json-patch": "^3.0.0-1",
"isomorphic-form-data": "~2.0.0",
"form-data-encoder": "^1.0.1",
"formdata-node": "^3.6.2",
"js-yaml": "^3.14.0",
"lodash": "^4.17.19",
"qs": "^6.9.4",
Expand Down
5 changes: 5 additions & 0 deletions src/http/fold-formdata-to-request.browser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const foldFormDataToRequest = (formdata, request) => {
request.body = formdata;
};

export default foldFormDataToRequest;
29 changes: 29 additions & 0 deletions src/http/fold-formdata-to-request.node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { Readable } from 'stream';
import { Encoder } from 'form-data-encoder';

/**
* formdata-node works in node-fetch@2.x via form-data-encoder only.
* FormData instance is converted to Encoder instance which gets converted
* to Readable Stream.
*
* TODO(vladimir.gorej@gmail.com): this can be removed when migrated to node-fetch@3.x
*/
const foldFormDataToRequest = (formdata, request) => {
const encoder = new Encoder(formdata);
const readableStream = Readable.from(encoder);

// get rid of previous headers
delete request.headers['content-type'];
delete request.headers['Content-Type'];

// set computed headers
request.headers = { ...request.headers, ...encoder.headers };

// set FormData instance to request for debugging purposes
request.formdata = formdata;

// assign readable stream as request body
request.body = readableStream;
};

export default foldFormDataToRequest;
13 changes: 8 additions & 5 deletions src/http.js → src/http/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import jsYaml from 'js-yaml';
import pick from 'lodash/pick';
import isFunction from 'lodash/isFunction';
import { Buffer } from 'buffer';
import { FormData } from 'formdata-node';

import FormData from './internal/form-data-monkey-patch';
import { encodeDisallowedCharacters } from './execute/oas3/style-serializer';
import { encodeDisallowedCharacters } from '../execute/oas3/style-serializer';
import foldFormDataToRequest from './fold-formdata-to-request.node';

// For testing
export const self = {
Expand Down Expand Up @@ -50,9 +51,10 @@ export default async function http(url, request = {}) {
}

// for content-type=multipart\/form-data remove content-type from request before fetch
// so that correct one with `boundary` is set
// so that correct one with `boundary` is set when request body is different than boundary encoded string
const contentType = request.headers['content-type'] || request.headers['Content-Type'];
if (/multipart\/form-data/i.test(contentType)) {
// TODO(vladimir.gorej@gmail.com): assertion of FormData instance can be removed when migrated to node-fetch@2.x
if (/multipart\/form-data/i.test(contentType) && request.body instanceof FormData) {
delete request.headers['content-type'];
delete request.headers['Content-Type'];
}
Expand Down Expand Up @@ -393,7 +395,8 @@ export function mergeInQueryOrForm(req = {}) {
const contentType = req.headers['content-type'] || req.headers['Content-Type'];

if (hasFile || /multipart\/form-data/i.test(contentType)) {
req.body = buildFormData(req.form);
const formdata = buildFormData(req.form);
foldFormDataToRequest(formdata, req);
} else {
req.body = encodeFormOrQuery(form);
}
Expand Down
62 changes: 0 additions & 62 deletions src/internal/form-data-monkey-patch.js

This file was deleted.

10 changes: 6 additions & 4 deletions test/execute/main.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import FormData from '../../src/internal/form-data-monkey-patch';
import { Readable } from 'stream';

import { execute, buildRequest, self as stubs } from '../../src/execute';
import { normalizeSwagger } from '../../src/helpers';

Expand Down Expand Up @@ -701,14 +702,15 @@ describe('execute', () => {

// Then
expect(req.headers).toEqual({
'Content-Type': 'multipart/form-data',
'Content-Length': 134,
'Content-Type': expect.stringMatching(/^multipart\/form-data/),
});

// Would like to do a more thourough test ( ie: ensure the value `foo` exists..
// but I don't feel like attacking the interals of the node pollyfill
// for FormData, as it seems to be missing `.get()`)
expect(req.url).toEqual('http://swagger.io/one');
expect(req.body).toBeInstanceOf(FormData);
expect(req.body).toBeInstanceOf(Readable);
});

test('should add Content-Type application/x-www-form-urlencoded when in: formData ', () => {
Expand Down Expand Up @@ -1190,7 +1192,7 @@ describe('execute', () => {
});

expect(fetchSpy.mock.calls.length).toEqual(1);
expect(fetchSpy.mock.calls[0][0].body).toBeInstanceOf(FormData);
expect(fetchSpy.mock.calls[0][0].body).toBeInstanceOf(Readable);
});

describe('parameterBuilders', () => {
Expand Down
Loading