Hi,
As a newbie, a real working demo is much better than pseudocode. I literally have no idea how new Job() in the example is implemented.

Instead of doing this:

const progressPromise = new PProgress((resolve, reject, progress) => {
	const job = new Job();

	job.on('data', data => {
		progress(data.length / job.totalSize);
	});

	job.on('finish', resolve);
	job.on('error', reject);
});

The API could instead be reworked to do this:

const progressPromise = pProgress(async progress => {
	const job = new Job();

	job.on('data', data => {
		progress(data.length / job.totalSize);
	});

	job.on('error', error => {
		throw error
	});

	return await pEvent(job, 'finish')
})

With the legacy Promise interface working like this:

const progressPromise = pProgress(progress => new Promise((resolve, reject) => {
	const job = new Job();

	job.on('data', data => {
		progress(data.length / job.totalSize);
	});

	job.on('finish', resolve);
	job.on('error', reject);
}))

It looks like PProgress.fn already does something similar but you still have to invoke the returned function:

I don't see how it is possible to implement concurrency, it says it is possible in options, but practically, consider following example:

import PProgress from 'p-progress'
import delay from 'delay'

const one = async function() {
    console.log("one started.")
    await delay(1000)
    console.log("one finished.")
}
const two = async function () {
    console.log("two started.")
    await delay(1000)
    console.log("two finished.")
}


const x = PProgress.all([
  async function() { console.log("WOW");await one()},
  () => two(),
  delay(3000),
  delay(1000)], {concurrency: 1});

(async function() {
    x.onProgress(console.log)
    console.log(new Date())
    await x
    console.log(new Date())
})()

functions one and two will both not execute at all.

if you put them in array as [one(), two()] then, concurrency will not be honoured, they will execute at the same time.

Hi,

I think there is a wrong type definition for PProgress.all method.

PProgress.all should return a PProgress<[AllValuesType]> instead of a PProgress<AllValuesType>

Could you fix it?

Thanks

Current implementation of forcing us to report the progress fraction has a lot of limitations:

1. Hides the done and total values data from callbacks. This data might be useful in various implementations.
2. Takes away the ability to handle progress of jobs that don't know about the total value. Some operations just don't know the totalSize, but could still report the progress of how much data has been processed thus far.
3. Inaccurate progress reporting in PProgress.all(). Without p-progress, or callback consumers knowing the totals, each job is equal no matter how much data it is processing.

I propose not enforcing the passing of fraction, but instead keeping track of both done and total values, reporting them, and letting the user decide how to handle it. This will give us more options and data in our progress implementations.

API:

// current
progress(done / total);
// proposal
progress(done, [total]);

The onProgress callback would than report these values in 1st and 2nd argument. If total is undefined, the end is unknown and user can switch handling:

progressPromise.onProgress((done, total) => {
  if (total === undefined) {
    console.log(`${done} bytes processed thus far`);
  } else {
    console.log(`${(done / total) * 100}% done`);
  }
});

In PProgress.all().onProgress callback, the done and total values in onProgress callback will be the total additions of both of the values of all passes progress promisses. And if any of the promisses is missing the total value, the resulting onProgress callback will also receive undefined as total, since we can no longer tell what the total of all promisses is now.

Huge benefit of knowing about the total total in PProgress.all() callback is a more accurate calculation of the whole progress. For example, if you have 2 jobs, one is processing 1KB, another is processing 99KB, and the 1st ends while the 2nd hasn't even started, you now have an accurate progress of 0.01, instead of 0.5. And if you do want it to report that 0.5, well you just do this in each job progress report:

progress(done / total, 1);

And now each job is equal no matter of the amount of data it's processing.

As you see, this kind of API would give us a lot more power, options, data, and ability to handle jobs with unknown totals in our implementations, while still remaining very simple.

Example code:

const got = require("got");
const PProgress = require('p-progress');
const { createWriteStream } = require("fs");

const settings = require("../settings.json");

const url = "https://media0.giphy.com/media/4SS0kfzRqfBf2/giphy.gif";
const name = "image.gif";

const fileDownload = PProgress.fn(async (url, name, progress) => {
    const downloadStream = got.stream(url);
    const fileWriterStream = createWriteStream(`${settings.download_target}/${name}`);

    downloadStream
        .on("downloadProgress", ({ transferred, total, percent }) => {
            const percentage = Math.round(percent * 100);
            // console.log(percent);
			// progress(percent);
			// progress(0.01);
            progress(1/10);
            // console.error(`${name} progress: ${transferred}/${total} (${percentage}%)`);
        })
        .on("error", (error) => {
            console.error(`Download failed: ${error.message}`);
            throw new Error(`Download Failed: ${error.message}`);
        });

    fileWriterStream
        .on("error", (error) => {
            console.error(`Could not write file to system: ${error.message}`);
            throw new Error(`File Writing Failed: ${error.message}`);
        })
        .on("finish", () => {
            console.log(`File downloaded to ${name}`);
            return true;
        });

    downloadStream.pipe(fileWriterStream);
});

(async () => {
	const progressPromise = fileDownload(url, 'Gather rainbows');

	progressPromise.onProgress(console.log);

	await progressPromise;
})();

No matter what I've tried it always returns 1 a single time and then doesn't report anything again. Am I missing something?

Node: v14.15.1
Environment: Ubuntu in WSL2 on Windows 10 (application is run in Ubuntu though)

Recently there were updates somewhere and suddenly one of my CLI's stopped working because p-progress threw an error complaining that the reported percentage is lower than the last reported:

Idea

What if we changed it to do nothing if no bigger progress is reported (no matter it's smaller or equal than previous):

if (progress <= this._progress) {
	return;
}

this._progress = progress;

App would just skip unacceptable values until a bigger value is reported. Currently, API seems too strict.

What do you think?

Progress listeners chained after .then or .catch are not being called. Likely an issue with the this context.

Example reproduction:

const PProgress = require('p-progress');
const delay = require('delay');

const createPromise = () => new PProgress(async (resolve, reject, progress) => {
    progress(0.25);
    await delay(500);
    progress(0.75);
    await delay(500);
    resolve();
});

createPromise().onProgress(p => console.log('Bare', p));
createPromise().then(() => {}).onProgress(p => console.log('Then', p));
createPromise().catch(() => {}).onProgress(p => console.log('Catch', p));

// expected
Bare 0.25
Then 0.25
Catch 0.25
Bare 0.75
Then 0.75
Catch 0.75
Bare 1
Then 1
Catch 1

// actual
Bare 0.25
Bare 0.75
Bare 1
Then 1
Catch 1

Like https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled