Finalize hot-reloading feature #1773

samogot · 2018-08-20T20:20:39Z

Rename live-reload to hot-reload
Add debug logging about reloading
Add documentation
Version bump and changelog

- Get rid of `roughLibraryKeyDecode` - use library path exposed from dart runtime now. - Get load modules from cache directly and throw if it isn't loaded instead of using require.js and hoping that module already loaded and no network request would be done. - Don't mess with `.ddc` extension in `build_runner` code - encapsulate all of it on build_web_compilers side.

- Rename live-reload to hot-reload - Add debug logging about reloading - Add documentation - Version bump and changelog

grouma

Really excited about this.

Just a bunch of little nit pics due to documentation / grammar. You'll probably want someone else to give a pass through the documentation since I'm not the best at writing documentation myself.

grouma · 2018-08-20T20:59:13Z

build_runner/lib/src/server/hot_reload_client/reloading_manager.dart

            _reloadPage();
            _running.complete();
            return;
          }
          _dirtyModules.add(parentId);
        }
      }
+      print('$reloadedModules modules was hot-reloaded.');


[nit] modules were*

grouma · 2018-08-20T21:00:14Z

docs/hot_module_reloading.md

+## What is Hot Module Reloading?
+
+This is a feature that allows you to speed up development process by reducing time between writing
+code and seeing the result. It detects changes in your code and automatically reloads minimum amount


[nit] reloads the* minimum amount of* js modules

grouma · 2018-08-20T21:01:59Z

docs/hot_module_reloading.md

+objects closured, so reloading A itself doesnt invalidate closure. In basic configuration
+hot-reloading reloads all modules in chain from changed module up to the root module of applications
+and reruns `main()` function for changes to take effect. This means that without special measures
+state of your won't be preserved, nevertheless hot-reloading is much faster than full page


[nit] This means that without special measures the state* of your application* won't be preserved

grouma · 2018-08-20T21:08:43Z

docs/hot_module_reloading.md

+
+If something depends on changed module A, it may have some instances of A's types, functions and
+objects closured, so reloading A itself doesnt invalidate closure. In basic configuration
+hot-reloading reloads all modules in chain from changed module up to the root module of applications


[nit] Let's rephrase this a bit.

In a basic configuration, hot-reloading reloads all the modules from a changed module up to the root module of the application and reruns main()....

grouma · 2018-08-20T21:09:43Z

docs/hot_module_reloading.md

+hooks will be called to determinate, is it possible to handle it's reloading, or all parents (other
+modules that depends on this one) should be marked as invalidated too. To implement the hook all you
+need to do is to define a top-level publicly exported function with name and signature of a hook.
+This hooks are:


[nit] These* hooks

grouma · 2018-08-20T21:10:30Z

docs/hot_module_reloading.md

+
+Any object returned from this function will be passed to update hooks. Use
+it to save any state you need to be preserved between hot reloadings.
+Try do not use any custom types here, as it might prevent their code from


[nit] Try to not use*

grouma · 2018-08-20T21:12:21Z

docs/hot_module_reloading.md

+May return nullable bool. To indicate that reload completes successfully
+return `true`. To indicate that hot-reload is undoable return `false` - this
+will lead to full page reload. If `null` returned, reloading will be
+propagated to parent.


[nit] to the*

grouma · 2018-08-20T21:14:33Z

docs/hot_module_reloading.md

+DOM is not in the same state you application will assume it is. 
+
+To resolve this situation you need to implement `hot$onDestroy` hook, to restore the state of DOM
+your application expect. You may allso change your initial code to handle all possible stated of DOM,


[nit] typo also*

jakemac53 · 2018-08-20T20:38:57Z

build_runner/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.10.2
+
+- `--live-reload` cli option is replaced with `--hot-reload` one with appropriate


Should we maintain both of these as separate features? I think it might make sense as hot-reload might not work for everybody but live-reload should.

I'm not sure if it will be very useful, because hot-reload already falls back to live-reload in complicated situations. If we still need to maintain both, we need to find good way to pass this option to client-side code

can you elaborate on how and when the fallback happens? Do we have that documented somewhere?

There are three cases when page reload might happen:

Dependency graph is changed since original page load. Documented in Known issues

Bubbling up from parent. We have default implementation of hot$onChildUpdate hook, so in current implementation it can't really happen. Documented only in test cases.

Either hot$onChildUpdateor hot$onSelfUpdate returns false. Not really a fallback but rather feature. Documented in hooks description.

The default logic today won't be correct for all apps though - it doesn't always fall back on a full reload it just re-invokes main, which won't work for a lot of apps.

We could change that so it always falls back on a hot reload unless you implement some hooks though?

If we decide to remove the --live-reload flag then maybe we should just deprecate it instead, and log a warning that points at the --hot-reload flag.

Ok. I will return live-reload flag in next PR

jakemac53 · 2018-08-20T20:58:01Z

build_runner/lib/src/server/hot_reload_client/reloading_manager.dart

            _reloadPage();
            _running.complete();
            return;
          }
          _dirtyModules.add(parentId);
        }
      }
+      print('$reloadedModules modules was hot-reloaded.');


jakemac53 · 2018-08-20T21:00:34Z

build_web_compilers/CHANGELOG.md

@@ -2,6 +2,7 @@

 - Only call `window.postMessage` during initialization if the current context
  is a `Window`.
+- Added more javascript code to dev bootstrap for hot-reloading support


I think this version has already been published, so we need a new ## 0.4.4-dev heading here (and should update the pubspec as well)

jakemac53 · 2018-08-20T21:02:09Z

docs/hot_module_reloading.md

+## What is Hot Module Reloading?
+
+This is a feature that allows you to speed up development process by reducing time between writing
+code and seeing the result. It detects changes in your code and automatically reloads minimum amount


nit: the minimum amount of

jakemac53 · 2018-08-20T21:03:29Z

docs/hot_module_reloading.md

+objects closured, so reloading A itself doesnt invalidate closure. In basic configuration
+hot-reloading reloads all modules in chain from changed module up to the root module of applications
+and reruns `main()` function for changes to take effect. This means that without special measures
+state of your won't be preserved, nevertheless hot-reloading is much faster than full page


nit: your application

jakemac53 · 2018-08-20T21:42:25Z

docs/hot_module_reloading.md

+### Handling reloading of child modules
+
+Lets assume you have a builder that transforms your css files into dart code exporting the string.
+As this string doesn't have any impact on your logic, you want to handle reloading of this modules,


nit: these modules

jakemac53 · 2018-08-20T21:42:54Z

docs/hot_module_reloading.md

+As this string doesn't have any impact on your logic, you want to handle reloading of this modules,
+to prevent parent from reloading
+
+For simplicity of example, lets assume we have `addCss` and `removeCss` methods thad do real DOM


nit: To simplify the example

jakemac53 · 2018-08-20T21:44:44Z

docs/hot_module_reloading.md

+to prevent parent from reloading
+
+For simplicity of example, lets assume we have `addCss` and `removeCss` methods thad do real DOM
+modifications. In this example it will just operate with `Set`.


nit: In this example we will just add and remove the styles from a Set.

jakemac53 · 2018-08-20T21:47:07Z

docs/hot_module_reloading.md

+  such bundled modules won't work - code will be executed, but parent module will still be reloaded.
+  That happens because current requirement is for all libraries in module to know how to handle
+  child updates. If you actively use this hook, you may consider turning on `fine` build strategy in
+  your `build.yaml`, work around this issue. But this will also slow down your builds. [#1767](https://github.com/dart-lang/build/issues/1767)


nit: to work around this issue

jakemac53 · 2018-08-20T21:48:29Z

docs/hot_module_reloading.md

+  your `build.yaml`, work around this issue. But this will also slow down your builds. [#1767](https://github.com/dart-lang/build/issues/1767)
+
+  ```yaml
+  global_options:


Most people probably only need to actually enable this for the root package, I think it is probably worth showing how to do both. The config for a regular package is like this:

targets: $default: builders: build_modules|modules: options: strategy: fine

# Conflicts: # build_runner/lib/src/server/hot_reload_client/client.dart.js # build_runner/lib/src/server/hot_reload_client/client.dart.js.map

* Add assets digests handler (#1709) This entry point will be used for hot reloading, in order to initially retrieve digests of all assets, as build_runner might not know list of all of them, unlike client. * Emit useful information about build results (#1710) Instead of just 'update' emit json with updated build results and its digests * Move live reload client code to new hot_reload_client.dart file (#1732) It is expected that client code will become complicated, so in separate dart file it would be easier to test and maintain. * Implement basic hot reload (#1741) Reload any changed modules without any specific order, then reload main module and call main func to hopefully apply changes. Still need to get module dependency graph to reload in topological order and find correct ancessor chain where reloaded modules can be saved in closures. * Basic graph handling for hot reload (#1759) We still lack handling situations when graph is dynamically updated between reloads. * Add rough graph changes handling by full page reload (#1763) * Simplify module interface (#1764) * Change HMR API to work with several libraries bundled in one module (#1765) Fixes #1762 * Close Hot Reload listeners on exit (#1775) Closes #1774 * Refactor and improve HMR code (#1770) - Get rid of `roughLibraryKeyDecode` - use library path exposed from dart runtime now. - Get load modules from cache directly and throw if it isn't loaded instead of using require.js and hoping that module already loaded and no network request would be done. - Don't mess with `.ddc` extension in `build_runner` code - encapsulate all of it on build_web_compilers side. Fixes #1760 Fixes #1762 * Finalize hot-reloading feature (#1773) - Rename live-reload to hot-reload - Add debug logging about reloading - Add documentation - Version bump and changelog Fixes #1766 * Restore --live-reload option (#1778)

samogot added 2 commits August 17, 2018 11:06

Fix some nits

eeb6575

googlebot added the cla: yes Google is happy with the PR contributors label Aug 20, 2018

samogot requested review from jakemac53 and natebosch August 20, 2018 20:20

samogot added 2 commits August 20, 2018 13:43

Add comment

06a4d42

Finalize hot-reloading feature

a931c37

- Rename live-reload to hot-reload - Add debug logging about reloading - Add documentation - Version bump and changelog

samogot force-pushed the finalize-hot-reloading branch from f993dcb to a931c37 Compare August 20, 2018 20:43

grouma approved these changes Aug 20, 2018

View reviewed changes

Fix typos and grammar

8af506f

jakemac53 reviewed Aug 20, 2018

View reviewed changes

samogot added 2 commits August 20, 2018 15:49

Fix more nits

a9857e3

Fix formatting

c86ba11

samogot changed the base branch from refactor to hot-reload August 21, 2018 18:21

Merge branch 'hot-reload' into finalize-hot-reloading

72c2985

# Conflicts: # build_runner/lib/src/server/hot_reload_client/client.dart.js # build_runner/lib/src/server/hot_reload_client/client.dart.js.map

jakemac53 approved these changes Aug 21, 2018

View reviewed changes

samogot merged commit 0e9fd42 into hot-reload Aug 21, 2018

samogot deleted the finalize-hot-reloading branch August 21, 2018 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finalize hot-reloading feature #1773

Finalize hot-reloading feature #1773

samogot commented Aug 20, 2018 •

edited

Loading

grouma left a comment

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

grouma Aug 20, 2018

jakemac53 Aug 20, 2018

samogot Aug 20, 2018

natebosch Aug 20, 2018

samogot Aug 20, 2018

jakemac53 Aug 21, 2018

samogot Aug 21, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

jakemac53 Aug 20, 2018

		@@ -1,3 +1,9 @@
		## 0.10.2

		- `--live-reload` cli option is replaced with `--hot-reload` one with appropriate

Finalize hot-reloading feature #1773

Finalize hot-reloading feature #1773

Conversation

samogot commented Aug 20, 2018 • edited Loading

grouma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samogot commented Aug 20, 2018 •

edited

Loading