Simple Fetch API Wrapper with Concise File Upload

https://github.com/WebsiteBeaver/far-fetch

There's many advantages this wrapper has over similar ones, but I will highlight file uploading.

async uploadFiles() {
  const photos = document.querySelector('#photos').files;
  const videos = document.querySelector('#videos').files;

  await ff.put('https://example.com/upload', {
    data: { name: 'Jeremy' },
    files: { photos, videos },
  });
}

Code (markup):

Doing this in vanilla fetch would require creating a new FormData object, traverse files in separate arrays and then append it to the FormData, followed by appending any request data needed. Then this would be passed into the body as a stringified object, along with the header content type being set to JSON.

This is so much extra work that could be saved by using FarFetch, which aims to simplify tasks like this.

 

Some observations:

1) The use of "import", "..." parameters, and arrow functions makes it not "real world deployable" for those of us with clients who still have to support IE 11. This is less and less of a problem, but it's still a concern for many. Hell, the mere presence of arrow functions will crash scripting site-wide in IE... and for what? Encouraging slower scripting through the use of unneccessary callbacks and "wah wah, eye duns wunna type teh wurd function"? (not a fan)

2) It's called SWITCH/CASE, use it.

3) If more than half your non-minified codebase "needs" to be block comments, there's probably something horribly wrong with both your code and your commenting.

4) don't introduce the overhead of multiple methods if they all do the same thing and are one-liners. Yeah, yeah "but it's easier to say ".get()" instead of ".patch(bunchostuff, 'get')"... the overhead of the excess function calls would likely beg to differ.

5) when possible, put your "..." parameters at the END, not the beginning.

Real laugh is I have the nasty feeling this could be done simpler with less code creating a wrapper for XMLHttpRequest, than it does using "await", but that's been my overall experience with the shoe-horning of this "promise" trash into JavaScript any-old-way.

 

I appreciate you taking the time to give me an honest critique.

> "The use of "import", "..." parameters, and arrow functions makes it not "real world deployable" for those of us with clients who still have to support IE 11. This is less and less of a problem, but it's still a concern for many. Hell, the mere presence of arrow functions will crash scripting site-wide in IE... and for what? Encouraging slower scripting through the use of unneccessary callbacks and "wah wah, eye duns wunna type teh wurd function"? (not a fan)"

Yes, this class if definitely not for anyone who needs IE support, as it doesn't support Fetch API itself . Axios is better for that and I don't view myself as competitor to it at all. Different classes for different use cases. I personally don't need IE support for instance, but clearly you do.

> "It's called SWITCH/CASE, use it"

I can take a look at the code and consider switching some ifs to switch/case.

> "don't introduce the overhead of multiple methods if they all do the same thing and are one-liners. Yeah, yeah "but it's easier to say ".get()" instead of ".patch(bunchostuff, 'get')"... the overhead of the excess function calls would likely beg to differ."

The normal `fetch()` method is available as well.

> "when possible, put your '...' parameters at the END, not the beginning."

I took a look at my code again and they are at the end. Except for if I'm calling `.post()` or `.get()`, which off course should override other options. Like no one should call `put()`, but then change the method in the options, right?

> "Real laugh is I have the nasty feeling this could be done simpler with less code creating a wrapper for XMLHttpRequest, than it does using "await", but that's been my overall experience with the shoe-horning of this "promise" trash into JavaScript any-old-way".

I can respect your opinion. Every programmer has his own preferences. This class is designed to be tailored to my preferences, but obviously not yours and that's ok.

Thank you again for taking the time to write a review.

 

The one that stood out most to me was:

 if (method === 'GET' || method === 'HEAD') {
      action = 'fetching';
    } else if (method === 'POST') {
      action = 'adding';
    } else if (method === 'PUT' || method === 'PATCH') {
      action = 'updating';
    } else if (method === 'DELETE') {
      action = 'deleting';
    }

Code (markup):

If they're all checking the same variable for ===, don't waste time else if else if else if.

switch (method) {
	case 'GET':
	case 'HEAD':
		action = 'fetching';
		break;
	case 'POST':
		action = 'adding';
		break;
	case 'PUT':
	case 'PATCH':
		action = 'updating';
		break;
	case 'DELETE':
		action = 'deleting';
		break;
}

Code (markup):

Though there were a couple other spots.

... and if anyone or anything (like a "linter") tells you not to use drop-through on cases like that, I've got two words for them.



You could probably also benefit from not wasting time introducing thrashing with CONST on things that don't need to be static references, NOT declaring let/var/const when you have multiples of them in a row, and NOT wasting memory and time on "variables for nothing".

For example:

	static async modifiedResponse(response) {
		var
			responseContentType = response.headers?.get('Content-Type'),
			modifiedResponse = response.clone();
		if (responseContentType?.includes('application/json')) {
			modifiedResponse.responseJSON = await response.json();
			modifiedResponse.responseText = null;
		} else if (responseContentType?.includes('text/plain')) {
			modifiedResponse.responseJSON = null;
			modifiedResponse.responseText = await response.text();
		}
		return modifiedResponse;
	} // modifiedResponse

Code (markup):

Whilst LET and CONST have theoretical corner case uses, they're slower, wasteful, and give garbage collection fits. I mean in this case you didn't need to override hoisting, you don't need them to be static instances, and in some cases you didn't even need them to be variables. If you only use it once, don't waste a var/let/const on it.

 

Oh also, whilst they SEEM like they're a cleaner format, as a rule of thumb .foreach with arrow functions are an epic /FAIL/ at coding, since they are so much more painfully slower (by adding functions for nothing) over a conventional loop... and typically end up more code due to verbosity.

For example where you have:

   Object.keys(files).forEach((key) => {
        const propFiles = files[key]; // Each object property representing distinct file category

        if (propFiles instanceof File) { // Single, unnamed file
          const propFile = propFiles; // Set to be more readable and consistent, as it's singular

          formData.append(key, propFile);
        } else if (Array.isArray(propFiles)) { // Array of files for object property
          propFiles.forEach((propFile) => {
            formData.append(`${key}[]`, propFile);
          });
        }
      });

Code (markup):

I would STILL do this:

// I'm assuming the instanceOf Array version would never have loose false entries
for (var key in files) {
	var value = files[key];
	if (value instanceOf File) formdata.append(value);
	else if (value instanceOf Array) {
		for (var j = 0, propFile; propFile = value[j]; j++) {
			formData.append(`${key}[]`, propFile);
		}
	}
}

Code (markup):

Because it's less code, clearer code, executes MANY times faster, and has backwards compatibility. It's a perfect example of why I consider a LOT of the new syntactic sugar to be complete and utter junk.

As evidenced once equivalently formatted and comment stripped the original being 311 bytes, the "old way" being 251 bytes whilst executing many MANY times faster. (assuming all indexes of files[key] are loose true). .foreach, arrow functions, and so forth have really left me not just underwhelmed, but questioning the competence of the people creating this trash.

 

Thank you for sharing your insight.

I actually looked backed and felt that was the only that should be changed even before you wrote that comment, so looks like we're in agreement on that. Can you point to me where else you think there are other examples of ones that should be converted to switch/case?

Very interesting take I don't normally hear. I just prefer using const when possible, for the same reason I use the most secure file permissions I can. I figured, do the most restrictive as you can. I'll probably even convert to TypeScript soon.

I prefer the newer method for the reason of not having to check for hasOwnProperty(), but I'll definitely give all of your opinions careful thought.