build icon indicating copy to clipboard operation
build copied to clipboard

Proposal: Add `--hot-reload` flag to `pub run build_runner run` to support hot-reloading

Open thosakwe opened this issue 7 years ago • 12 comments

If https://github.com/dart-lang/build/pull/1629 is good enough to get merged, then I think eventually people might want to be able to pass a --watch (or --hot) flag to the command, so that on file changes, their application could reload in-place, a là Flutter.

I'd be glad to send a PR for this, especially since it's something I think a lot of people could make good use of.

Pros

  • Faster edit-refresh cycle, as a new build script wouldn't have to be generated every time.
  • Based on functionality that already exists in the VM service + watch command
  • It's cool 😉

Cons

  • Requires a new dependency: package:vm_service_lib
  • Difficult to unit test
  • New arguments cannot be passed to the script between reloads (though if you wanted to do this you'd probably just start a new process anyway)

Example

Tobes-MacBook-Air:tobe thosakwe$ pbr run bin/hello.dart --hot -- a b --c --d=efg
[INFO] Generating build script completed, took 695ms
[INFO] Reading cached asset graph completed, took 162ms
[INFO] Checking for updates since last build completed, took 535ms
[WARNING] Invalidating asset graph due to build script update
[INFO] Cleaning up outputs from previous builds. completed, took 10ms
[INFO] Building new asset graph completed, took 553ms
[INFO] Checking for unexpected pre-existing outputs. completed, took 2ms
[INFO] Running build completed, took 4.7s
[INFO] Caching finalized dependency graph completed, took 66ms
[INFO] Creating merged output dir `/var/folders/md/xrrg3dn921l5_qw7v2l3cb9w0000gn/T/build_runner_run_scriptQooPfA/` completed, took 1.9s
[INFO] Writing asset manifest completed, took 11ms
[INFO] Succeeded after 6.7s with 2 outputs (3 actions)

<script output will appear here>
Args: [a, b, --c, --d=efg]

<this is after a file change>
[INFO] ------------------------------------------------------------------------
[INFO] Starting Build
[INFO] Updating asset graph completed, took 5ms
[INFO] Running build completed, took 5.4s
[INFO] Caching finalized dependency graph completed, took 21ms
[INFO] Succeeded after 5.5s with 2 outputs (3 actions)

<on success>
Performing hot reload...
Reloaded 1 of 448 libraries in 2,777ms.

<output of script will appear here>
Args: [a, b, --c, --d=efg]

<on failure>
Hot reload was rejected, because 1 or more builds failed:
  * example|bin/hello.dart
  * example|bin/main.dart

thosakwe avatar Jul 01 '18 22:07 thosakwe

Experimental version: https://github.com/thosakwe/build/tree/experimental-hot-reloading

thosakwe avatar Jul 02 '18 00:07 thosakwe

Given that most applications can't necessarily be gracefully "hot reloaded", what is the goal here?

For example, a server listening to :8080 in main() will fail when hot reloaded, right?

matanlurey avatar Jul 06 '18 17:07 matanlurey

I tested this out just a bit - which was fun as I haven't had the ability to dive into hot reload outside of flutter until now!

The primary issue I ran into (trying to edit a trivial server similar to what @matanlurey highlighted), is the need for some sort of hook to know when a hot reload starts/stops.

For my quick example I hacked it up to provide events on stdin for the spawned process, but we probably would want something more robust.

What do you think about spawning it as an isolate instead of a separate process, and passing that a ReceivePort in to the main method? Then the controlling process can send messages on hot reload start/end and we don't have to worry about dealing with stdin/stdout. I don't know if the main process would have to then be launched with these special flags and whatnot, or if that can be enabled explicitly for the new isolate?

jakemac53 avatar Jul 06 '18 17:07 jakemac53

For example, a server listening to :8080 in main() will fail when hot reloaded, right?

fwiw @matanlurey this seems to work just fine - the main thing is that only methods seem to get reloaded so you can't use any instance/static/const fields, or you need a hook to reset the state of your app if you do.

jakemac53 avatar Jul 06 '18 18:07 jakemac53

What do you think about spawning it as an isolate instead of a separate process, and passing that a ReceivePort in to the main method? Then the controlling process can send messages on hot reload start/end and we don't have to worry about dealing with stdin/stdout.

I think this would be a good solution; I guess we would just have to clearly define what sort of message would be sent along the SendPort.

It would be cool to have a JSON-RPC sort of API where scripts are told exactly what was reloaded, but maybe that might be overkill.

theSendPort.send({'type': 'reload', 'sources': ['example|my_file.dart', 'example|some_other_file.dart']});

We would have to have a ReceivePort listening in each Isolate, but IMO that might be fine.

thosakwe avatar Sep 13 '18 18:09 thosakwe

In my head it would make some amount of sense to expose some sort of API, so that you could do something like this, matching the API from serve:

import 'package:build_runner/build_runner.dart';

main(List<String> args, [SendPort sendPort]) async {
  if (sendPort != null) {
    var listener = new HotReloadListener(sendPort);
     listener.onDestroy = () => ...; // cleanup
     listener.onSelfUpdate = ([data]) => ...;
     listener.onChildUpdate = (childPath, child, [data]) => ...;
     await listener.start();
  }
} 

thosakwe avatar Nov 07 '18 20:11 thosakwe

I don't know if the main process would have to then be launched with these special flags and whatnot, or if that can be enabled explicitly for the new isolate?

I also forgot to mention - the main process does have to be launched with either --observe or --enable-vm-service, so I guess we either have to spawn it as a new Process, or, figure out some way to enable the VM service within an already-running VM.

thosakwe avatar Nov 07 '18 20:11 thosakwe

@grouma @jakemac53 @natebosch – we have this more or less w/ webdev right?

kevmoo avatar Apr 02 '19 00:04 kevmoo

We don't have stateful hot reload in webdev just yet. I don't have the full context of this thread however I believe this is adding hot reload support to running VM applications which webdev obviously doesn't support.

grouma avatar Apr 02 '19 16:04 grouma

I read over the solution a little more. This is pretty neat! I'm not sure that it makes sense to live in build_runner though. We are in the process of moving the hot reload logic out of the package and into webdev. If webdev supported running VM applications I think we'd be able to fairly easy support this use case.

grouma avatar Apr 02 '19 16:04 grouma

I see that serve is going to be removed from build_runner (https://github.com/dart-lang/build/issues/2227), so yeah, it might be better to just implement this in a separate tool.

thosakwe avatar May 01 '19 08:05 thosakwe

I think it is an open question for sure, with the build daemon now it is easier to make a separate tool which can invoke builds and coordinate with other tools that also care about builds running at the same time.

It seems weird for this support to be a part of webdev, as that is pretty specific to web projects which typically wouldn't have vm apps mixed in with them.

I actually wouldn't be that opposed to this support it living in build_runner... but I would also be happy to see a separate package that supports it.

I am also curious if hot reload can work with precompiled dill files or not? I think flutter does actually do it that way though so it probably works.

jakemac53 avatar May 01 '19 13:05 jakemac53

we don't support build_vm_compilers any more

jakemac53 avatar Sep 01 '23 18:09 jakemac53