loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.07k forks source link

[CLI] Watch generated projects #1234

Closed virkt25 closed 3 years ago

virkt25 commented 6 years ago

Description / Steps to reproduce / Feature proposal

Projects scaffolded and started using the CLI have a start script but any changes such as adding new controllers requires us to manually restart the server. This should be automated using tsc watch mode imo similar to how angular does it for it's projects.

I would propose to even go as far as creating a lb4 serve / lb4 start command similar to angular that can accomplish the watch mode for development purposes while npm start can be without watch mode for use in production.

Current Behavior

Restart the app manually when a new controller is added.

Expected Behavior

App restarts automatically once a new controller has been added to the project.

dhmlau commented 6 years ago

Closing in favor of https://github.com/strongloop/loopback-next/issues/1259, since that issue has more detailed description.

virkt25 commented 6 years ago

This task is related but not the same ... I would say this is blocked by #1259 not a duplicate. This is a CLI improvement using the implementation of #1259

This is a proposal to add a lb4 start or lb4 serve command which starts the LB4 project with watch mode enabled (implemented in #1259).

virkt25 commented 6 years ago

If it was decided we don't want to implement this at all I'm ok to close it (with a message saying that).

joeytwiddle commented 6 years ago

This is how I ~do it~ used to do it: (For how I do it now, see below)

"start:watch": "nodemon -e 'js,ts' -w src -d 2 -x 'npm start || exit 1'",

"build:watch": "npm run clean && lb-tsc es2017 --outDir dist --watch",
"start:watch-light": "nodemon -e '*' -w dist -d 2 -x 'node dist/server.js || exit 1'",

The first one (start:watch) is slow, because npm start does a full rebuild.

The second approach uses the incremental compiler, but I do have to run two separate commands! (I use two terminal panes for that.) But it is certainly worthwhile: development goes much faster.

Occasionally the contents of dist will end up in a bad state. (I think this happens when a source file is removed: the corresponding file doesn't get removed from dist. If so, switching to and from older branches could trigger it.) That's why I added clean to the build:watch script. If something breaks I just restart build:watch to get a clean slate.

I also have a test:watch-light script which watches dist, and re-runs the tests whenever it changes.

It would be great to have all (three?) tasks in one command, but at least this setup is working well for us right now.

virkt25 commented 6 years ago

@joeytwiddle Projects scaffolded using the lb4 CLI already have a build:watch script. Would you be interested in submitting a PR to add a start:watch to package.json scaffolded using the CLI since you have experience in this area?

joeytwiddle commented 6 years ago

Yes you are right, I only tweaked the existing build:watch very slightly.

I would be happy to do that.

But are you devs happy to add the extra nodemon dependency to the project?

Edit: Or perhaps tsc-watch would be preferable to nodemon. I haven't tried it yet.

virkt25 commented 6 years ago

I think as long as the CLI scaffolding the app gives an option to have watch powered by either nodemon / tsc-watch it should be ok (similar to prettier / lb-build / tslint) BUT I would like to let others chime in on the issue to get their thoughts on adding a new dependency.

cc: @strongloop/sq-lb-apex @strongloop/loopback-next @raymondfeng @bajtos

bajtos commented 6 years ago

I like that with tsc-watch, one needs only one terminal to see the output of both tsc and the application. The trouble is that tsc-watch does not work with our auto-selection of compilation target and dist folder depending on the current Node.js version (e.g. on Node.js 8.x, we use --target es2017 --outDir dist8).

To address this limitation, I am proposing to integrate tsc-watch into our lb-tsc helper:

Alternatively, we can introduce a new command lb-tsc-watch which can share a lot of the implementation code with lb-tsc, but execute tsc-watch instead of tsc.

Either way, by exposing tsc-watch functionality through a helper script we control, it will be easier in the future to fix possible bugs of tsc-watch in case the maintainers of tsc-watch are not responsive.


Regarding nodemon: I believe our CLI tooling should not be offering unnecessary many options to our users - Decision Fatigue is a real thing. I personally prefer to offer LB users only one solution, the one which we consider as the best.

I don't have a strong opinion whether this solution should be nodemon, tsc-watch or something else.

Adding an option allowing users to opt-out of watch mode (and thus a dependency on nodemon) in a fashion similar to how they can opt-out of prettier/lb-build/tslint integration is fine with me.

bajtos commented 6 years ago

FWIW, I think the implementation of tsc-watch could be simplified by using TypeScript Compiler API instead of calling out to tsc --watch script and parsing the script output. I am also not sure why tsc-watch executes system tools like taskkill and ps-tree to kill child processes (source) instead of using Node.js' process.kill() API - that's another source of extra dependencies.

What I am trying to say: if we decide to use tsc-watch and the additional dependency becomes a problem, then it should reasonably easy to refactor our wrapper to drop the dependency on tsc-watch and implement the functionality directly.

bajtos commented 5 years ago

I like that with tsc-watch, one needs only one terminal to see the output of both tsc and the application. The trouble is that tsc-watch does not work with our auto-selection of compilation target and dist folder depending on the current Node.js version (e.g. on Node.js 8.x, we use --target es2017 --outDir dist8).

FWIW, auto-selection of compilation target & dist folder is no longer a problem, we have removed this feature while preparing for TypeScript Project References & Build mode - see https://github.com/strongloop/loopback-next/pull/1803

stale[bot] commented 4 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

joeytwiddle commented 4 years ago

The current release of loopback seems to use tsconfig.tsbuildinfo to cache previously compiled hashes, making update builds much faster than the old full rebuild.

As a result, I have stopped using a separate watcher, and I now do everything in one process:

    "prestart": "npm run build",

    // Loopback's bundled start script, which does not respond to changes
    "start": "node -r source-map-support/register .",

    // Use start:watch instead. After a change it will trigger prestart and start
    "start:watch": "nodemon -e '*' -w src -d 2 -x 'npm run start' || exit 1",

This takes a few seconds to update the build and restart the server, but I find the simplicity (only needing one terminal window) is worth the price.

I do the same with test:watch which is really useful for our developers to watch while they are working.

(Something similar might be possible with tsc --watch but I haven't tried it.)


I'm also not wedded to nodemon. I am just sharing a technique that I know works, for people who want something right now. If we can achieve the same functionality a tidier way, I'm all for it!

There are also a few alternatives to nodemon, for example three were described in this 2013 Strongloop blog post. 🙂

dhmlau commented 4 years ago

@bajtos had a proposal in https://github.com/strongloop/loopback-next/issues/1234#issuecomment-403448059. @raymondfeng, any thoughts on which option to adopt to go forward? Thanks.

madaky commented 4 years ago

@dhmlau, @bajtos : As per my observation the global trends says that most of the developer use nodemon for watch configuration of their projects, Also in this particular thread and other thread related to debug and watch most of the developer found easy integration with nodemon. So far I also agree with @bajtos regarding own implementation for

lb-tsc --watch --onSuccess

in looback itself which may be a future feature with lbs' own watcher. we may put it for feature for future.

bajtos commented 4 years ago

I like that with tsc-watch, one needs only one terminal to see the output of both tsc and the application. The trouble is that tsc-watch does not work with our auto-selection of compilation target and dist folder depending on the current Node.js version (e.g. on Node.js 8.x, we use --target es2017 --outDir dist8).

Please note the comment above is no longer true. At the moment, lb-tsc is very tiny wrapper around tsc, it mostly finds the right tsconfig.json file and automatically decides whether to use build mode (-b) for composite projects using project references.

@madaky I think tsc-watch should work in LoopBack projects out of the box now. No need to implement our own version.

You can also use the solution described by @joeytwiddle in https://github.com/strongloop/loopback-next/issues/1234#issuecomment-604424514.

I guess the next step is to decide which solution we want to promote as "the right LoopBack way" and then update our CLI project templates to setup that solution for new projects, i.e. add "start:watch" script after "start":

https://github.com/strongloop/loopback-next/blob/6fbf23d3e6366a99ee0ff543fd05fe15bdb711ca/packages/cli/generators/project/templates/package.json.ejs#L99

mbnoimi commented 3 years ago

I guess the next step is to decide which solution we want to promote as "the right LoopBack way" and then update our CLI project templates to setup that solution for new projects, i.e. add "start:watch" script after "start":

Please take the decision; This issue opened in 2018! I'm still learning LB4 and I already miss the feature. I was surprised why I can't execute npm run start:watch in LB4!