Should minimumWait be maximumWait ?

[iwestlin]

exit-hook/index.js

Line 46 in 34078d9

forceAfter = Math.max(forceAfter, wait);

Node will exit at (or before) forceAfter ms, so I think it should called maximumWait.

[sindresorhus]

// @jakobo

[jakobo]

exit-hook/index.js

Line 46 in 34078d9

forceAfter = Math.max(forceAfter, wait);

Node will exit at (or before) forceAfter ms, so I think it should called maximumWait.

Consider the following:

• hookA registers asynchronously with a 100ms wait time
• hookB registers asynchronously with a 300ms wait time

At registration, this is the minimum time the hook will be waiting for. In the original PR we went back and forth on the name. I agree it feels confusing (even still).

I see two solutions, one of which would bump the major version

1. (4.0) Rename to maximumWait or better, just wait, further clarify the docs about how waiting works
2. (3.x) Rename to maximumWait / wait, alias minimumWait and mark as deprecated

I'm leaning towards the first option. In 2 weeks, node lts rotates (Sept 11 specifically). I'd prefer to bundle this change with the end of node16 support & encourage people to stay on top of supported node versions.

(Edit. From 📱)

[jakobo]

@iwestlin Draft PR is at #33 if you'd like to take a look; it specifically addresses option 1, moving the minimum required node version to 18 and renaming to wait + adds docs for clarity. I'll move it out of draft tomorrow. I prefer to sit on a change like this for a day so I can reread the diff with fresh eyes and minimize PR review time for Sindre.

[iwestlin]

At registration, this is the minimum time the hook will be waiting for.

Hi, I'm not sure what does minimum mean, even if there is only one registered hook with forceAfter waiting time, shouldn't it still be the maximum time node will be wating for?

        // Force exit if we exceeded our wait value
	const asyncTimer = setTimeout(() => {
		done(true);
	}, forceAfter);

	await Promise.all(promises);
	clearTimeout(asyncTimer);
	done();

• If the task in promises finished before forceAfter ms, the done() will be called and Node exits;
• If the task in promises takes longer than forceAfter ms, done(true) in setTimeout will be called at forceAfter ms and Node exits, leaving task unfinished.

All in all, the maximum waiting time will be forceAfter ms. Did I miss something? Could you elaborate on where did the minimum come from?

[iwestlin]

Ah, I think I get it...

When one hook was added with forceAfter waiting time, it doesn't know if there were or will be added more hooks with other waiting times, because of forceAfter = Math.max(forceAfter, wait), it only know the final value will be greater than or equal to forceAfter, so it's actually minimumMaximumWait...😂

I agree it's very confusing, simply call it wait might be a good idea. Or maybe we can export a setOptions function, rather than set options on every asyncExitHook call?

[jakobo]

Or maybe we can export a setOptions function, rather than set options on every asyncExitHook call?

You could, but that would shift the responsibility for knowing each hook's timeout to the developer calling gracefulExit() instead of the developer designing the hook. Who determines the wait time was another major design decision; I opted for PoLP design which kept gracefulExit()'s signature simple. At the very least, moving to wait with a more complete explanation should eliminate confusion. New docs read:

The amount of time in milliseconds that the onExit function is expected to take. When multiple async handlers are registered, the longest wait time will be used.

[sindresorhus]

Fixed by #33