npkill: Version 0.7.3 throws an error "Callback must be a function" on delete.

Hey @KonradNojman ! Good point regarding the flicker bug. I think i have found the cause: now the delete method in windows is synchronous, so the UI will ‘freeze’ while performing the delete job. Then, when the process finishes, it updates the status to “deleting…” and a few ms later, it changes to “deleted”.

I’ve added a small commit to #88 to fix this and refresh the screen before starting the process.

Hi guys!

Sorry for the delay. I have access to my machines again. I’m going to check everything 😃

Thanks so much @ognjenjevremovic for all the details and for the PR, and @KonradNojman being able to test it and give so much feedback. I’m going to check everything and hopefully I’ll be able to launch these changes in a while!

Hi @ognjenjevremovic @zaldih ! Sorry for the delay 😅 Finally I had a chance to check out the changes for this issue. Works awesome ✅ 🚀

I’ve reviewed the PR and accepted it after testing on Windows. Please, @zaldih also check the code as I’m not that familiar with the codebase and I could miss something important. 💪

---

When testing I think I maybe found one or 2 new bugs, so I’ll check if they already have issues created for them, and if not I’ll try to gather more data on them and post soon 😉

Hey @ognjenjevremovic ! First of all, thank you very much for your interest! It’s appreciated 😃

Currently it looks like no one has gotten around to it, so if you want it, it’s all yours. I’m going to tell you all the information I have (so anyone else can discuss options as well).

The problem resides only in this method https://github.com/voidcosmos/npkill/blob/9067388656502c2d281ae9bec780ed56584cc1bd/src/services/windows-files.service.ts#L40 , so to fix this problem it will not be necessary to touch any other file.

The method stopped working just in this change 81ad353e5b2430cb62a126aa8e329375e539305e .

Why change it if it was working? Because it seems that this method had problems with symbolic links, described in the issue mentioned a few messages ago.

Why does it fail now? I am almost certain that the new method is relatively new and does not work with some versions of node.

The fastest and most efficient solution would be to use some library like ‘RIMRAF’ , but we have tried to avoid using as many libraries as possible to make npkill as ‘independent’ as possible, so it will remain as a last option. But at this point, if nobody can’t think of a different solution, it will be better to use it and implement it 😄.

And to know if the problem is solved, you should try in windows to delete several node_modules with different versions of node (for example, v10, v12, v14 and v15). And make sure that the problem described in #81 does not occur again, following the steps described there.

Again, thanks for the interest, to you and everyone else for participating in any way in the project ❤️

Awesome 🚀 I hope I have some time today after work to quickly test your commit and let you know if that did the trick.

I think the one should be fine, although the more the merrier 🙂 . However, only versions below 12.10.0 are affected with this change (on Windows platform).

I tested it on LTS, v15.3.0 and v10.15.3. However, on a POSIX operating system (with a few tweaks in the code in order to reproduce the issue, as I do not have access to a win32 platform atm).

I think versions below v10 can be ignored, as the support for version 10 and below is about to expire later this year (in April). But it wouldn’t hurt if you have ways of testing it on more than one version of Node.

Feel free to ping me if you need any assistance testing this 🙂 .

@KonradNojman @suman-saket I just made a PR that addresses this issue https://github.com/voidcosmos/npkill/pull/88. If you are running Windows platform and would like to check the changes before (and if) they make it to a release, you may test the non-release branch locally, by:

• cloning the repository,
• checking out 4fbbb83 revision,
• running npm i && npm run build from the cloned repository,
• installing it from the local file system.

If you would like to test it on a dummy project (which I would strongly advise you to, before the code makes it to a production release), you can setup the environment (if you completed the previously outlined steps) by:

• creating a new directory and initializing it as an npm project by running mkdir test-npkill && cd test-npkill && npm run init -y,
• making a few subdirectories and repeating the process of initializing a new npm project inside them mkdir a && cd a && npm init -y as well as installing some dependencies to a project (any would work - this is just so you have a node_modules inside of those subdirectories),
• repeating the previous process a few times but making directories b, c, d as siblings to directory a,
• going back to the root project test-npkill and installing the non-release version (local version you previously build) with included changes from the 4fbbb83 HEAD by running npm i /absolute/path/to/the/cloned/npm/git/project/directory.

Of course these steps are more of a guideline rather than the steps to test / reproduce. As long as you use the 4fbbb83 HEAD with the included changes and running the local version of npkill tool from a directory that has node_modules as children you’ll be able to run the CLI without issues 🙂 .

Cheers @KonradNojman. That sounds wonderful (and it would be a great time saver)! 🚀 🙂 I’ll try post an update sometime during next week, when I hope I’ll have more info regarding this.

Hi @ognjenjevremovic! Nice to see you are interested in this bug 🚀 If you don’t find any way of testing your changes on Windows you can ping me. If I have some time, maybe I will be able to checkout your branch and test it out locally. Not promising anything, because I’m a little swamped with work right now, but I’ll try to help you out if needed 🙌 🙂