feat: strongly type event emitter methods

[43081j]

Adds strong types to the event emitter interface such that on and similar methods can infer parameters.

For example:

// these are strongly typed and will give hints in editors etc
watcher.on('ready', () => {});
watcher.on('error', (err) => {});

// this will fail
watcher.on('nonsense', () => {});

Fixes #1372

cc @paulmillr

[paulmillr]

Is that OK for backwards compat?

[43081j]

Sorry I forgot to reply to this. We do export it so it's technically a breaking change to the types. But as far as I'm aware, it's not part of our "public API".

I couldn't find any usages of it in the wild. So I think it's really just an internal type exported for completeness

[paulmillr]

Huh?

[43081j]

args used to contain the event name too and we basically didn't make use of event even though we passed it

I simplified so we use the event name and the args separately

We only had them combined for storing in the pending unlinks map. Everywhere else, we already had the name separately or didn't need it

i.e args[0] === event in the existing code

[ArnaudBarre]

I'm waiting on this PR to upgrade on some of my projects, is a release planned?

[43081j]

cc @paulmillr

i think we just need to cut a release of main, as we haven't done one in a while and have since merged a few things

[mattjohnsonpint]

FYI, this was a breaking change for us.
https://github.com/hypermodeinc/modus/actions/runs/12397272157/job/34607246349

Fixed with hypermodeinc/modus@680f422

[paulmillr]

Why is the first argument error now @43081j ?

[mattjohnsonpint]

For me, it was string and now it's string | Error. I'm not sure in what circumstance an error would occur but the event would still be raised?

[43081j]

it was always string | Error under the hood but the event handlers weren't strongly typed, so it was never enforced

this is because the generic code for emitting events can sometimes pass an error, and unlink doesn't have an explicit signature so falls back to that

we should probably just add unlink to the explicit ones here:

chokidar/src/index.ts

Lines 301 to 306 in 69c115a

	export interface FSWatcherKnownEventMap {
	[EV.READY]: [];
	[EV.RAW]: Parameters<WatchHandlers['rawEmitter']>;
	[EV.ERROR]: Parameters<WatchHandlers['errHandler']>;
	[EV.ALL]: [EventName, ...EmitArgs];
	}

as something like:

[EV.UNLINK]: [string]

there may be other handlers in the same situation.

a quick scan around tells me unlink and unlink_dir will always be a path (never an error)

meanwhile, it seems like add and change can be an Error (maybe others too)

[43081j]

i've opened #1397 to at least fix the unlink events

there may be other events but we can look into that separately

[mattjohnsonpint]

Makes sense. Any suggestions for how to handle such errors? I think in my case, ignoring them is fine. Maybe some specific examples of the kinds of errors that could be passed would be helpful to others. Thanks.

[paulmillr]

meanwhile, it seems like add and change can be an Error (maybe others too)

How so? It was my assumption from day one it's always path string.

[43081j]

i think its mostly in situations where awaitWriteFinish is set. if while polling the file, it vanishes or a stat fails, that error will be passed to the callback

i suspect you can mostly ignore them if that's the case (i.e. just skip your handler if it is an error)

[paulmillr]

Can you produce a test example?

[43081j]

its difficult to rep;roduce, but its basically here:

chokidar/src/index.ts

Lines 637 to 651 in 69c115a

	const awfEmit = (err?: Error, stats?: Stats) => {
	if (err) {
	event = EV.ERROR;
	args[0] = err;
	this.emitWithAll(event, args);
	} else if (stats) {
	// if stats doesn't exist the file must have been deleted
	if (args.length > 1) {
	args[1] = stats;
	} else {
	args.push(stats);
	}
	this.emitWithAll(event, args);
	}
	};

this gets called over time (polling) as it waits for the file size to settle

it is checking the file size each time via a stat call. so if one of those calls fails, or the file has been removed, it will pass an error back here:

chokidar/src/index.ts

Lines 770 to 773 in 69c115a

	if (err || !writes.has(path)) {
	if (err && err.code !== 'ENOENT') awfEmit(err);
	return;
	}

edit:

i'm an idiot - it also sets the event type to error! so this one won't actually emit an error, it changes the event entirely

[43081j]

this Path | Error stuff was here before but it seems wrong now that i look at it

ill chop the code up a bit and see where i get to

[43081j]

@paulmillr i've updated #1397 to try fix this, please take a look if you can