-
Notifications
You must be signed in to change notification settings - Fork 28.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nodemon (or similar) in core #40429
Comments
I very much prefer https://github.com/fgnass/node-dev to nodemon mostly because nodemon restarts the whole process when it detects a change, whereas node-dev only reloads the single module which changed and it's dependents. On bigger projects this can save quite a lot of overhead. |
@capaj that's a different use case |
@boneskull why do you think it's a different usecase? I use it on typical backend projects interchangeably. |
it appears to be for development. but regardless, this issue is about nodemon, which many people use |
Are you talking about using nodemon as a tool to restart your server in production? FYI most people use nodemon just for development. A very tiny minority of users use nodemon for other usecases. |
I would like to start that we should look at solving the major use cases, not all possible edge cases with this. I do want this feature very badly.
I think determining what restart means (soft vs hard) would be a good first step to figuring out what could be in core. I think if we do want devtools integration that relies on reloads like perf tab to properly work we likely won't be able to just vendor in an existing library though. Also, node can likely spin up a much smaller footprint manager than a full blown JS VM. I don't think bickering about gritty details and if something is the best solution needs to be done at the investigation phase of this. I think there will generally be more specialized solutions that people can use if whatever node ships doesn't fit their specific need. We are trying to help users and if we don't ship anything due to bickering, we don't improve the situation for anyone. |
This feature request seems like one of those things that can be very opinionated, which makes me lean towards -1 for inclusion in node core. However, I suppose at the very least if it's possible to build/install node without whatever tool, I would be more ok with it. I already use (other) 3rd party tools for managing node processes and don't want to add more bloat to my node installations. |
@mscdex I'm not sure preventing every kilobyte of unused core is really a goal of Node. Providing use cases to users is paramount and various things like existing devtools workflows that are currently disabled in the node inspector need some kind of internal reload manager. We for example ship |
We don't already include nodemon, so these examples aren't quite the same. Besides, as far as I understand it, in the case of acorn and websockets both were more or less required. nodemon is not required to fix an issue or compatibility problem that currently exists in node core itself.
That's a bit extreme. We've already had this small core discussion many times in the past, so I'm not going to repeat it here. However, I don't think there is anything wrong with wanting a small(er) node core (not just in literal size but also in scope) and not wanting to throw in the kitchen sink. |
I tend to agree we need something to solve this problem. I don't think we should be embedding nodemon - that's a solved problem at this point, I don't think we would stop people from using nodemon. I would like something better than nodemon. However the hot reloading within devtools and |
due to some limitations in the current node ESM loader implementation? |
As far as I know it's currently impossible to implement hot module reloading in ESM. |
No. This is due to how caching is constrained by the specification, once a URL is used it is permanently cached and cannot be unloaded.
In both cases there is no requirement that node support users wishing to use the features these provide. |
@bnb a watch mechanism could automatically propagate an |
Well, you can reload... but it leaks memory and under the hood rewrites URLs (would still be true with any sort of --watch), we have a PR with a test to show that mocking/aliasing a URL works but the only way to not leak is to have some sort of manager like reloading and an opt-out mechanism like import.meta.hot to avoid unnecessary reloading. |
ping on this. |
@bnb what's the next step? |
I think agreement on scope would be a good start. Having a process manager
vs having a workflow to reload in the same process is one scope argument.
Having solutions for C++ add-ons, WASM, and workers/cluster for scoping in
both cases likely differ greatly. After scoping, determining mechanisms to
instrument reloading is likely a secondary concern.
…On Tue, Feb 1, 2022, 12:26 PM Tierney Cyren ***@***.***> wrote:
my understanding of next steps:
it (generally) seems like there's agreement that we'd like to see
*something* along these lines. defining the scope of what that is and
isn't, including the feedback from @mcollina <https://github.com/mcollina>
and @bmeck <https://github.com/bmeck>, would be helpful in guiding an
actual eventual implementation.
—
Reply to this email directly, view it on GitHub
<#40429 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABZJI77KXIQQ6BG5ERHIH3UZAQUDANCNFSM5F3DLGJQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@bnb a preliminary step is to embed https://www.npmjs.com/package/fsevents in core to replace the buggy fs watch logic we have for macs (might be a new API). There might be another issue around this somewhere. I'm happy to join a call to scope this. |
@mcollina the documentation says that we use fsevents on mac: https://nodejs.org/dist/latest-v17.x/docs/api/fs.html#availability What is buggy about what we do? Is there an issue about it? |
I don't know, maybe @pipobscure can help clarify. |
The latest libuv does use fsevents under the hood these days. This was an effort undertaken by libuv a while back after chatting about moving fsevents into node core at a conference (again quite a while back). This wasn’t always the case. Originally libuv was using posix stuff for monitoring files and would quickly run into ulimit for file handle if monitoring deep directories (one handle per directory/file). So for tools like nodemon I’d recommend moving to the builtin fs watch capabilities as they should be sufficient. Fsevents offers more granularity and more history, so it’s still useful though for a much smaller number of cases. It’s the underlying technology for time machine, so you can actually go back in time (before process start) and recall events as well. Which is great, but not all that useful for something like nodemon. So from my perspective there’s nothing that would prevent just using the builtin stuff here. That being said, I have not tried out the fsevents implementation if libuv, so it may be worth doing an experiment. But if something doesn’t work quite right I’d think the place to fix it is in libuv. |
Thanks @pipobscure for the explanation! |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
@mcollina does @pipobscure's context help with the necessary step of using fsevents in core? |
A common ask, which I think needs a sub process (but I could be wrong), is when devs use nodemon with |
Could you elaborate on that point? |
if we use sub-processes we might as well vendor nodemon |
We also have a watcher in It also spawns a child process which has been working great. No noticeable performance drawbacks and I think the implementation is minimal & maintainable. Another file watcher alternative to consider is Apparently it was built to address the unreliability of chokidar and is now even used by VSCode. |
@MoLow nice. Now wondering if nodemon just got it's death sentence 😆 😢 |
@remy I don't think so. First, this has a long way to go before reaching feature parity (i.e. https://github.com/remy/nodemon/blob/HEAD/doc/requireable.md is missing). Second, this is hardly stable, and it will require a few years before it is rolled out to all Node.js versions. Third, everybody using your software would hardly switch. |
:D yeah, the old dogs and new tricks thing will probably keep folks using nodemon for many a year. Good addition to node either way 👍 |
PR-URL: #44366 Fixes: #40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> (cherry picked from commit beb0520)
PR-URL: nodejs#44366 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44571 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #44366 Backport-PR-URL: #44571 Fixes: #40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: #44366 Backport-PR-URL: #44571 Fixes: #40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44814 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44815 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #44366 Backport-PR-URL: #44815 Fixes: #40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: https://github.com/nodejs/node/pull/TBD Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44877 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44877 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44976 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44976 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#44366 Backport-PR-URL: nodejs#44976 Fixes: nodejs#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #44366 Backport-PR-URL: #44976 Fixes: #40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#44366 Backport-PR-URL: nodejs/node#44976 Fixes: nodejs/node#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#44366 Backport-PR-URL: nodejs/node#44976 Fixes: nodejs/node#40429 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Is your feature request related to a problem? Please describe.
Not particularly. It's related to a problem in that it is proposing the addition or inclusion of a tool that is widely used for a common problem.
Describe the solution you'd like
I recently put out a tweet from @nodejs on Twitter asking folks what tool they find most useful day-to-day. I generally try to do this not with the intent to bring a proposal or something like that, but to have some engaging activity that could be helpful to someone somewhere.
One thing I noticed was just how many people replied with "nodemon". It seemed to come up just as often as DevTools, VS Code, and TypeScript which I found remarkable.
I'd like to recommend that we consider either vendoring nodemon or explore writing a nodemon-like from scratch in core.
Describe alternatives you've considered
Doing nothing, which would be fine. This is a problem that is obviously solvable in userland, but one that people clearly need solved. If the result of this issue is "this should be in userland" that's totally fine.
The text was updated successfully, but these errors were encountered: