nodemon icon indicating copy to clipboard operation
nodemon copied to clipboard

Watch cjs modules by default

Open lorand-horvath opened this issue 2 years ago • 2 comments

https://github.com/remy/nodemon/issues/1954

lorand-horvath avatar Nov 18 '21 08:11 lorand-horvath

@remy

Justman10000 avatar Sep 16 '22 16:09 Justman10000

I'm worried this change looks simple but possibly hiding impact, such as on people's defaults or on what happens ordinarily without it.

I don't have the time ATM to give it a good test, but it's obviously easy to add this to your own projects to work around it.

remy avatar Sep 16 '22 17:09 remy

I've moved this in to the latest release. It should be nodemon@3.

remy avatar Jul 07 '23 09:07 remy

@remy Sorry for having to say this upfront, but I was very much expecting for you to actually merge my PR, which was waiting for you to make up your mind and include *.cjs files as default watch since November 2022. For some reason you refused initially, but now all of a sudden it's fine to use it. Instead of merging and linking my PR, all you did was thank me with a general @ in your commit https://github.com/remy/nodemon/commit/86d5f403a3e06e8aed48b37fa854730dc83257be which will be untraceable within a few days already.

I know it's not a big deal, but that's not how open source acknowledgements are normally carried out and I think it discourages people from collaborating or supporting each other.

I will completely uninstall nodemon from my dev machines at this point and just use node 18 --watch mode. I strongly urge others to do the same.

If you think I have made a mistake and perhaps am missing something here, please let me know.

lorand-horvath avatar Jul 07 '23 15:07 lorand-horvath

It wasn't really all of a sudden! But it was because it was barely a change. The actual work, that you didn't do in the pr, which is why I was wary, was the testing.

This is what I did on Friday, I created a series of simple test cases and ran them against the change.

It's always the tests that show the work.

But, please do uninstall nodemon. Using node 18's native watch gets you pretty far, I'm sure it'll serve you well (and do share the word about nodes native reload, it helps others so that's not a bad thing).

Hopefully that helps you understand where the real work was, and why I wasn't sure whether it was a safe pr to merge (without having to do the leg work myself back in Novem).

remy avatar Jul 08 '23 18:07 remy

For what it's worth, I didn't manage to fully test this (because I don't use typescript at all), someone caught that it broke default watching for TS: https://github.com/remy/nodemon/issues/2124

So I baked the cjs watching directly into the code, seen here: https://github.com/remy/nodemon/commit/95bee008bfb4eb77d7826f193e9386812652f449#diff-d649d3b2f2afcc1dfc4d0dce7c28b4fc296860b45dd38d3c7bb0274a6be73708L117 (and then of course tested again)

remy avatar Jul 09 '23 08:07 remy