foss42 / apidash

API Dash is a beautiful open-source cross-platform (macOS, Windows, Linux, Android & iOS) API Client built using Flutter which can help you easily create & customize your API requests, visually inspect responses and generate API integration code. A lightweight alternative to postman/insomnia.
https://apidash.dev
Apache License 2.0
1.68k stars 324 forks source link

Broken Javascript Codegens for Multipart requests. #293

Closed Tanish2002 closed 8 months ago

Tanish2002 commented 8 months ago

Describe the bug/problem Fetch and Axios codegens generate wrong not working code for multipart form type requests.

Expected behavior Correct Code is generated for any type of request.

Tanish2002 commented 8 months ago

since #286 was closed. I'll continue the discussion here.

I can easily fix the nodejs codegen.

The plain client-side js one is a bit tricky. Since js runs on the browser it doesn't have access to the filesystem so it can read the files from filepaths.

I checked out how Postman and insomnia both generate it and both are wrong. Where insomnia is simply wrong(won't work) and Postman has a bug.

Insomnia one:

const form = new FormData();
form.append("text", "some text");
form.append("file1", "/home/weeb/Wallpapers_repo/terminator.jpg");
form.append("file2", "/home/weeb/Wallpapers_repo/Mocha.png");

const options = {
  method: 'POST',
  headers: {
    'Content-Type': 'multipart/form-data; boundary=---011000010111000001101001',
    'User-Agent': 'insomnia/2023.5.8'
  }
};

options.body = form;

fetch('https://api.apidash.dev/io/img', options)
  .then(response => response.json())
  .then(response => console.log(response))
  .catch(err => console.error(err));

Here file1 and file2 don't work as "files" instead just plain strings. so the server will just receive strings

form.append("file1", "/home/weeb/Wallpapers_repo/terminator.jpg");
form.append("file2", "/home/weeb/Wallpapers_repo/Mocha.png");

Postman:

var formdata = new FormData();
formdata.append("text", "some text");
formdata.append("file", fileInput.files[0], "terminator.jpg");
formdata.append("file2", fileInput.files[0], "Mocha.png");

var requestOptions = {
  method: 'GET',
  body: formdata,
  redirect: 'follow'
};

fetch("https://api.apidash.dev/io/img", requestOptions)
  .then(response => response.text())
  .then(result => console.log(result))
  .catch(error => console.log('error', error));

Here as you can see both form fields have value of fileInput.files[0] when it should be fileInput.files[0], fileInput.files[1].

formdata.append("file", fileInput.files[0], "terminator.jpg");
formdata.append("file2", fileInput.files[0], "Mocha.png");

Also one more big thing in postman. fileInput is never explained to the user. A new user with very less experience is js would not know what fileInput is supposed to mean.

It's supposed to be a input element that is selected. something like this

<input type="file" id="fileInput">
const fileInput = document.getElementById('fileInput');
Tanish2002 commented 8 months ago

An actual complete example of working client side JS fetch code would be this:

const fileInput = document.getElementById('fileInput'); // <input type="file" id="fileInput">
const uploadButton = document.getElementById('uploadButton'); // <button id="uploadButton">Upload</button>

uploadButton.addEventListener('click', () => {
  function buildDataList(fields) {
    const formdata = new FormData();
    for (const field of fields) {
      const name = field.name || '';
      const value = field.value || '';
      const type = field.type || 'text';
      let fileCounter = 0;

      if (type === 'text') {
        formdata.append(name, value);
      } else if (type === 'file') {
        formdata.append(name, fileInput.files[fileCounter], value);
        fileCounter++
      }
    }
    return formdata;
  }

  const payload = buildDataList([{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }]);
  const url = 'https://api.apidash.dev/io/img';

  const options = {
    method: 'POST',
    body: payload
  };

  fetch(url, options)
    .then(res => {
      console.log(res.status);
      return res.json()
    })
    .then(body => {
      console.log(body);
    })
    .catch(err => {
      console.error(`error:${err}`);
    });
});

This has code that should be easy to understand for beginners.

Tanish2002 commented 8 months ago

quoting discord chats:

Sure you can open a new issue. I understand your query. The issue with client side js is that we do not have access to file system. We can assume fileInput is available in html and just add a comment with a link to the github discussion with more detailed steps on integrating the code.

I think something like this should work fine? @ashitaprasad

// refer to <insert some link> for details regarding integration.
function buildDataList(fields) {
  const formdata = new FormData();
  for (const field of fields) {
    const name = field.name || '';
    const value = field.value || '';
    const type = field.type || 'text';
    let fileCounter = 0;

    if (type === 'text') {
      formdata.append(name, value);
    } else if (type === 'file') {
      formdata.append(name, fileInput.files[fileCounter], value);
      fileCounter++
    }
  }
  return formdata;
}

const payload = buildDataList([{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }]);
const url = 'https://api.apidash.dev/io/img';

const options = {
  method: 'POST',
  body: payload
};

fetch(url, options)
  .then(res => {
    console.log(res.status);
    return res.json()
  })
  .then(body => {
    console.log(body);
  })
  .catch(err => {
    console.error(`error:${err}`);
  });
animator commented 8 months ago

The above looks good @Tanish2002 if it works with the full html + js template provided in the integration link (you can post a template in this thread for us to try out and review). Also, I am thinking it would be much cleaner if we separate nodejs & js codegens as now significant differences are cropping up.

Tanish2002 commented 8 months ago

@animator, Hmm from what I've changed for now(currently working) I don't think we need to segregate nodejs and js versions. I'll open a PR first and if you think it's better we segregate them then I can do it sure.

Furthermore, Here is the requested complete example

Complete example HTML + Javascript ```html File Uploader

Simple File Uploader

```
The Actual Generated code. ```js // refer to for details regarding integration. const payload = new FormData(); payload.append("token", "xyz") payload.append("imfile", fileInput1.files[0]) payload.append("more files", fileInput2.files[0]) const url = 'https://api.apidash.dev/io/img'; const options = { method: 'POST', body: payload }; fetch(url, options) .then(res => { console.log(res.status); return res.text() }) .then(body => { console.log(body); }) .catch(err => { console.error(`error:${err}`); }); ```
animator commented 8 months ago

Sounds good 👍 Thanks for the example.

animator commented 8 months ago

Also one more thing @Tanish2002 as you are refactoring the js codegens. Please get rid of the buildDataList part that was included earlier to construct the multipart formdata as it is very crude.

function buildDataList(fields) {
  const formdata = new FormData();
  for (const field of fields) {
    const name = field.name || '';
    const value = field.value || '';
    const type = field.type || 'text';
    let fileCounter = 0;

    if (type === 'text') {
      formdata.append(name, value);
    } else if (type === 'file') {
      formdata.append(name, fileInput.files[fileCounter], value);
      fileCounter++
    }
  }
  return formdata;
}

const payload = buildDataList([{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }]);

Also, the [{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }] being passed is an internal json list of form model which should not have been exposed in the first place.

Kindly refactor the codes to be more like the Go (http) codegen

Tanish2002 commented 8 months ago

@animator More updates! I changed the way fetch codegen worked. I removed the buildDataList functions since I believe it was redundant, and didn't provide any actual value to the code generated.

I've refactored the codegen to look more sleek and simple. Please give your thoughts if I should continue with this or discard it.

JS Fetch Codegen ```js // refer https://github.com/foss42/apidash/issues/293#issuecomment-1994927844 for integration const payload = new FormData(); payload.append("token", "xyz") payload.append("imfile", fileInput.files[0]) payload.append("more files", fileInput.files[1]) const url = 'https://api.apidash.dev/io/img?size=2&len=3'; const options = { method: 'POST', headers: { "User-Agent": "Test Agent", "Keep-Alive": "true" }, body: payload }; fetch(url, options) .then(res => { console.log(res.status); return res.text() }) .then(body => { console.log(body); }) .catch(err => { console.error(`error:${err}`); }); ```
NodeJS Fetch Codegen ```js import fetch from 'node-fetch' import { fileFromSync, FormData } from 'node-fetch' const payload = new FormData(); payload.append("token", "xyz") payload.append("imfile", fileFromSync("/path/to/file.jpg")) payload.append("more files", fileFromSync("/path/to/file.png")) const url = 'https://api.apidash.dev/io/img?size=2&len=3'; const options = { method: 'POST', headers: { "User-Agent": "Test Agent", "Keep-Alive": "true" }, body: payload }; fetch(url, options) .then(res => { console.log(res.status); return res.text() }) .then(body => { console.log(body); }) .catch(err => { console.error(`error:${err}`); }); ```

If you agree with the changes, then I can edit comment accordingly

Tanish2002 commented 8 months ago

Also one more thing @Tanish2002 as you are refactoring the js codegens. Please get rid of the buildDataList part that was included earlier to construct the multipart formdata as it is very crude.

function buildDataList(fields) {
  const formdata = new FormData();
  for (const field of fields) {
    const name = field.name || '';
    const value = field.value || '';
    const type = field.type || 'text';
    let fileCounter = 0;

    if (type === 'text') {
      formdata.append(name, value);
    } else if (type === 'file') {
      formdata.append(name, fileInput.files[fileCounter], value);
      fileCounter++
    }
  }
  return formdata;
}

const payload = buildDataList([{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }]);

Also, the [{ "name": "token", "value": "xyz", "type": "text" }, { "name": "imfile", "value": "/home/weeb/screenshots/2023-12-27_14:21:15.png", "type": "file" }] being passed is an internal json list of form model which should not have been exposed in the first place.

Kindly refactor the codes to be more like the Go (http) codegen

haha we were thinking the same! Sure I already did that, will go ahead with this now!

animator commented 8 months ago

What a coincidence @Tanish2002 🙀

Tanish2002 commented 8 months ago

@animator Created a pr as well updated the comment

animator commented 8 months ago

As per our discussion above, I ran the file input code. It works perfectly, but I find one issue with our current approach. Usually if a form takes an input file usually it is a single file (like select to upload Id proof, select to upload address proof). Currently we are allowing users to select multiple files from a single file input button and assigning fields in the sequence of file (0 ,1, 2 ...) selected by the user:

        payload.append("imfile", fileInput.files[0])
        payload.append("more files", fileInput.files[1])

This is a bit ambiguous as the end user does not know that the sequence of file selection is important. Usually multiple file input is used whenever a bulk operation has to be performed on the files and the sequence of file selection is not important.

So, I recommend adding distinct single file input for each field and we should document the same. So it should be - fileInput1, fileInput2 and so on ....

Tanish2002 commented 8 months ago

So, I recommend adding distinct single file input for each field and we should document the same. So it should be - fileInput1, fileInput2 and so on ....

Ok so from what I understand I simply have to change the codegen to look like this:

payload.append("imfile", fileInput1.files[0])
payload.append("more files", fileInput2.files[0])

where each fileInput(num) is a input tag in html document. so something like this:

<input type="file" id="fileInput1">
<input type="file" id="fileInput2">
const fileInput1 = document.getElementById('fileInput1'); 
const fileInput2 = document.getElementById('fileInput2'); 
animator commented 8 months ago

yup @Tanish2002

Tanish2002 commented 8 months ago

Ok I'll make the required changes and update the comment as well

Tanish2002 commented 8 months ago

This comment is supposed to be documentation for axios client side integration.

Complete example HTML + Javascript (Axios) ```html File Uploader

Simple File Uploader

```
The Actual Generated Axios(JS) code. ```js // refer to for details regarding integration. const config = { url: 'https://api.apidash.dev/io/img', method: 'post', headers: { "Content-Type": "multipart/form-data" }, data: { "token": "xyz", "imfile": fileInput1.files[0], "more files": fileInput2.files[0] } }; axios(config) .then(res => { console.log(res.status); console.log(res.data); }) .catch(err => { console.log(err); }); ```