Version 4: Typescript, ESM, glob removal, drop node <18

[paulmillr]

• Remove globs
• Switch to ESM
• Switch to typescript
• Release readdirp v4
• Changelog
• Release chokidar v4

[sw360cab]

@paulmillr is this alpha release blocked ?
Is there any plan to release it? Maybe some contribution needed.

Thanks a lot for your great work.

[paulmillr]

Nah -- someone just needs to find some time to debug it.

[paulmillr]

We also need to make this ESM and upgrade https://github.com/paulmillr/readdirp/

[43081j]

We also need to make this ESM and upgrade https://github.com/paulmillr/readdirp/

i have a branch where i've tried to remove readdirp since node itself should be capable by now (with recursive: true)

though it has a few test failures so i was planning on trying to fix those

what do you think?

unrelated - the same failure as my last few PRs happened again here. seems to be windows specific 🤔 i'd like to solve that one somehow

[paulmillr]

ideally we'll just use node built-ins, ofc.

Try readdirp-ing 400K-file directory and seeing how's the speed with both solutions. I imagine nodejs is still shit. Keep in mind that readdirp uses streams etc.

[43081j]

thats true, i'll try it out on a larger project if i can find one some place 👍

[43081j]

i did some benchmarking, and it seems like readdirp recursively visits some symlinked directories, whereas node won't

in a test repo:

readdirp - Found 122112 items
node - Found 32967 items

it seems the missing items are already part of the 32967 (symlinked by pnpm in this case)

in a directory that doesn't hit that problem, node seems faster:

┌─────────┬────────────┬─────────┬────────────────────┬──────────┬─────────┐
│ (index) │ Task Name  │ ops/sec │ Average Time (ns)  │ Margin   │ Samples │
├─────────┼────────────┼─────────┼────────────────────┼──────────┼─────────┤
│ 0       │ 'readdirp' │ '9'     │ 105280229.69999999 │ '±9.32%' │ 10      │
│ 1       │ 'node'     │ '41'    │ 23874190.62790698  │ '±3.59%' │ 43      │
└─────────┴────────────┴─────────┴────────────────────┴──────────┴─────────┘

[paulmillr]

Symlinks are definitely important.

[43081j]

Symlinks are definitely important.

im not saying node is ignoring them. readdirp is duplicating them from what i can tell

readdirp - Found 122112 items
node - Found 32967 items

the extra 89,145 entries seem to be files in node_modules we already visited (already accounted for in the 32,967).

symlinks certainly are traversed via node

[benmccann]

@benmccann how flexible are you in terms of ESM-onliness?

Sorry I missed this question, but don't worry about me. I'm fine with whatever you decide either way

[43081j]

just fyi too the last two weeks have been fairly chaotic for me so i haven't had much time to look into whats left yet

but things should calm down this weekend for a week or two, so ill try catch back up on it. would be nice to at least sort the windows tests out

[43081j]

@paulmillr if you didn't see, it looks like the windows failures fail in master too

it does seem like it could be a bug but we could at least release a pre-release for now. that would be nice i think (4.0.0-pre.0 or something, in a next tag on npm)

the functionality seems pretty sound and i've been through the diff with master, which shows our only changes really are:

• removal of globs
• ignore logic

so should be good, and any problems we see are likely already problems (that we should fix)

[paulmillr]

I would still prefer to fix tests for v4. They have been broken for many years.

Do you think we can do it?

[43081j]

i agree

its a busy couple of days coming up but ill take a look again when i can and see if i can figure it out

[43081j]

@paulmillr it is a bug, i've figured it out roughly but not how to fix it

when we write a file (add.txt), we cause a rename fs event

this results in us calling the handleDir watcher here:

chokidar/src/nodefs-handler.ts

Lines 587 to 592 in c8c3ff2

	closer = this._watchWithNodeFs(dir, (dirPath, stats) => {
	// if current directory is removed, do nothing
	if (stats && stats.mtimeMs === 0) return;
	this._handleRead(dirPath, false, wh, target, dir, depth, throttler);
	});

that starts readdirp going on the directory:

chokidar/src/nodefs-handler.ts

Lines 483 to 488 in c8c3ff2

	let stream = this.fsw
	._readdirp(directory, {
	fileFilter: (entry) => wh.filterPath(entry),
	directoryFilter: (entry) => wh.filterDir(entry),
	depth: 0,
	})

which finds add.txt and adds the node:

chokidar/src/nodefs-handler.ts

Line 518 in c8c3ff2

this._addToNodeFs(path, initialAdd, wh, depth + 1);

we then reach this stat call (which is async):

chokidar/src/nodefs-handler.ts

Line 622 in c8c3ff2

const stats = await statMethods[wh.statMethod](wh.watchPath);

meanwhile, a change event fires - nothing really comes of this because we haven't finished our stat of the file, so haven't added a listener yet

when the test passes - a second change event fires at this point, and there's still no file listener so we do not fire a CHANGE event

now the stat finishes and adds the file listener to the set of listeners

when the test fails - the second change event fires at this point, and there is a file listener so we fire a CHANGE event

---

im not exactly sure what we can do about this

the TLDR of the problem is that the stat finishes before the followup change, so it thinks its a real change (but its not, windows at least fires 2 for whatever reason)

[43081j]

🤔

is it possible this is just wrong:

chokidar/src/nodefs-handler.ts

Line 385 in c8c3ff2

if (!at || at <= mt || mt !== prevStats.mtimeMs) {

specifically at <= mt:

• if the access time is before the modified time, it means we haven't accessed it since it was modified, so this is a change
• if the access time is the same as the modified time, surely it implies it hasn't changed?

if i change this to at < mt, the tests pass

edit:

i made #1335 but it breaks the reverse (ubuntu). ill take another look another time

[millette]

Related: #1336

[benjamingwynn]

Will this release address #1225, or will another issue have to be created?

[paulmillr]

New release drops support for globs. So, it would address it by setting it "out of scope". As in: all glob patterns would become unsupported. You will need to write regexps, or filtering functions.