v5-alpha: massive rewrite #1342
Conversation
| // How it works now (except readdir): | ||
| // - we process first event (first change) | ||
| // - there is multiple parallel changes which we throw away | ||
| // - all changes which happened when we processed first event is lost |
There was a problem hiding this comment.
fixing this would probably also solve the windows bug
just a recap of what that is/was:
writing a file in windows often fires two events, while it fires one on linux/mac
|
how are things going with this @paulmillr? will be out travelling soon but if you need a hand with anything, shout up and ill try chip in |
|
I will upload new commit soon and then pause for a bit. It's too complicated. Feel free to code it. |
|
ill take a look once you push 👍 if you can leave any comments where you think stuff needs figuring out too, that'd be helpful would be so nice for us to get this over the line some day |
|
@43081j I think we should release current work as v4 and then the further refactor in this pull request as v5 when it's ready. I want to make chokidar massively compatible for v4. Could you help me to understand what's the lowest version we support and why? Perhaps we'll use commonjs + esm hybrid for v4. |
|
that makes sense we really should fix the windows bug at least, though. since it is an actual bug to remind you what that is:
something along the lines of that. if we can fix it, i think v4 is all good as for lowest node version - it should still work fine on anything |
|
It seems like readdirp is used with depth: 0 which means it's not recursive. So we don't even need it directly. Maybe we should just remove it. |
|
I noticed this too and have a branch where I switched to using node's recursive flag instead (if at all). But it had a few failures since it switches the logic from streams to promises so is a chunky rewrite I agree we probably should do it at some point though |
|
readdirp provides a bunch of features:
just keep this in mind. I think we can do it quickly. |
Did it also happen in v3? |
|
I'm pretty sure it did since we haven't changed the logic around there We changed tests that started to pick it up but it existed before afaict |
|
@43081j can you do a sanity check on main branch? I want to release it. Perhaps i've missed something? The tests seem to be randomly failing again. Also, what were the improvements that you've added for the rewrite? |
|
I'll have a look. seems a fair few things changed since the last working CI run but mostly looks like formatting and types as for what changed in the v4 work, most of it was removing the need for fsevents. we also changed it to account for inodes changing in macos (it was only Linux before, but both OSes do it). then a few changes to how ignore logic works, to simplify it |
|
The CI seems to be failing because of timeouts, need to fix that as well |
No description provided.