axios-module icon indicating copy to clipboard operation
axios-module copied to clipboard

feat: allow $loading indicator throttling

Open simllll opened this issue 6 years ago • 8 comments

Problem: Right now the loading indicator is shown even for very short request. Which results in a "flashing" that looks more like a bug than a feature ;)

Reason & Solution: when using .set on $loading it shows immediately the loading indicator. (see nuxt-loading.vue) The indicator itself has a throttle property though (which is only checked on .start()). As long as we only have one request, there is no additional benefit of using .set directly, therefore we can rely on the throttle parameter by using start() instead.

Different approach: Another approach would be implementing our own "throttle" method which sets a timer on "onRequest" when it's not set) or currentRequests === 0), and removes the timer again onResponse when currentRequests <= 0. But then we need another config option for the throttling param, or can we access the one from nuxt.config's loading property somehow?

Regards Simon

simllll avatar May 13 '19 14:05 simllll

Codecov Report

Merging #247 into dev will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #247   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         1      1           
  Lines        27     27           
  Branches     12     12           
===================================
  Hits         27     27

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f744666...777b63e. Read the comment docs.

codecov[bot] avatar May 13 '19 14:05 codecov[bot]

Codecov Report

Merging #247 into dev will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #247   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         1      1           
  Lines        27     27           
  Branches     12     12           
===================================
  Hits         27     27

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f744666...5653c6c. Read the comment docs.

codecov[bot] avatar May 13 '19 15:05 codecov[bot]

I've seen there is also another PR hanging arround regarding this issue. I've solved this for me now by: 1.) disabling progress updates globally via nuxt.config:

	axios: {
		progress: false, 
       }

2.) using my own axios-enrichment plugin, which I have in place anyway where I added follwoing code: plugins/axios-progress.ts:

import axios from 'axios';

export default ({ app, $axios, store, res }) => {
	const noopLoading = {
		finish: () => {},
		start: () => {},
		fail: () => {},
		set: () => {}
	};

	const $loading = () =>
		process.client && window.$nuxt && window.$nuxt.$loading && window.$nuxt.$loading.set
			? window.$nuxt.$loading
			: noopLoading;

	let requestsRunning = 0;
	
	$axios.onResponse(response => {
		requestsRunning -= 1;
		if (requestsRunning <= 0) {
			requestsRunning = 0;
			$loading().finish();
		}
	});

	$axios.onRequest(config => {
		requestsRunning += 1;
		if (requestsRunning === 1) {
			$loading().start();
		}
	});

	$axios.onError(error => {
		requestsRunning -= 1;
		if (!axios.isCancel(error)) $loading().fail();

		if (requestsRunning <= 0) {
			requestsRunning = 0;
			$loading().finish();
		}
	});
};

dont forget to add this in nuxt.config as plugin: plugins: ['~/plugins/axios-progress']

hope it's helpful.

regards Simon

simllll avatar May 15 '19 22:05 simllll

Thanks, @simllll for this PR and sorry about late review/answer. I just left a minor comment about this change. I think what you have proposed in your second comment is more reasonable (only call to start() on the first request) but always update progress.

pi0 avatar Jun 05 '19 11:06 pi0

(ping @simllll)

TheAlexLichter avatar Jul 25 '19 13:07 TheAlexLichter

hi @manniL, my last comment is more like a different approach, by just disabling the module's indicator and set up my own. Would you like me to adapt these changes to the module itself?

I think what you have proposed in your second comment is more reasonable (only call to start() on the first request) but always update progress.

The problem with that is, as soon as we set the progress the bar appears, therefore this would need some special handling which I'm unsure if this is really necessary. The progress of one request is not computed anyway, as onResponse is only called at the end of a request. Therefore a real benefit of the "progress" is only existing if there are a lot of parallel requests running. (Or we find a way to update the progress bar for longer running requests more frequently (e.g. during uploads or file downloads...))

Regards Simon

simllll avatar Jul 25 '19 15:07 simllll

I would really think there would be a larger layer of abstraction between the loading component and it's integration with plugins. I would assume there would be a way for plugins to simply say, "hey nuxt, I'm loading 3 things.... now I'm loading 2 things.... now 1 etc..." and nuxt's loading component would figure out how to render that abstraction over time. I really don't think plugins should be talking directly to the loading component.

hecktarzuli avatar Nov 05 '19 02:11 hecktarzuli

any news?

gaodeng avatar May 28 '20 13:05 gaodeng