Skip to content

Finalize hot-reloading feature #1773

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Aug 21, 2018
Merged

Finalize hot-reloading feature #1773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
718597c
Refactor and improve HMR code
samogot Aug 17, 2018
eeb6575
Fix some nits
samogot Aug 20, 2018
06a4d42
Add comment
samogot Aug 20, 2018
a931c37
Finalize hot-reloading feature
samogot Aug 17, 2018
8af506f
Fix typos and grammar
samogot Aug 20, 2018
a9857e3
Fix more nits
samogot Aug 20, 2018
c86ba11
Fix formatting
samogot Aug 21, 2018
72c2985
Merge branch 'hot-reload' into finalize-hot-reloading
samogot Aug 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions build_runner/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.10.2

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three cases when page reload might happen:

  1. Dependency graph is changed since original page load. Documented in Known issues
  2. 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.
  3. Either hot$onChildUpdateor hot$onSelfUpdate returns false. Not really a fallback but rather feature. Documented in hooks description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

functionality. See [hot-module-reloading](../docs/hot_module_reloading.md) for more
info.

## 0.10.1+1

- Added better error handling when a socket is already in use in `serve` mode.
Expand Down
3 changes: 2 additions & 1 deletion build_runner/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ Some commands also have additional options:
##### serve

- `--hostname`: The host to run the server on.
- `--live-reload`: Enables automatic page reloading on rebuilds.
- `--hot-reload`: Enables automatic module reloading on rebuilds.
See [hot-module-reloading](../docs/hot_module_reloading.md) for more info.

Trailing args of the form `<directory>:<port>` are supported to customize what
directories are served, and on what ports.
Expand Down
8 changes: 4 additions & 4 deletions build_runner/lib/src/entrypoint/options.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import 'package:path/path.dart' as p;
const assumeTtyOption = 'assume-tty';
const defineOption = 'define';
const deleteFilesByDefaultOption = 'delete-conflicting-outputs';
const liveReloadOption = 'live-reload';
const hotReloadOption = 'hot-reload';
const logPerformanceOption = 'log-performance';
const logRequestsOption = 'log-requests';
const lowResourcesModeOption = 'low-resources-mode';
Expand Down Expand Up @@ -131,13 +131,13 @@ class SharedOptions {
/// Options specific to the `serve` command.
class ServeOptions extends SharedOptions {
final String hostName;
final bool liveReload;
final bool hotReload;
final bool logRequests;
final List<ServeTarget> serveTargets;

ServeOptions._({
@required this.hostName,
@required this.liveReload,
@required this.hotReload,
@required this.logRequests,
@required this.serveTargets,
@required bool assumeTty,
Expand Down Expand Up @@ -203,7 +203,7 @@ class ServeOptions extends SharedOptions {

return ServeOptions._(
hostName: argResults[hostnameOption] as String,
liveReload: argResults[liveReloadOption] as bool,
hotReload: argResults[hotReloadOption] as bool,
logRequests: argResults[logRequestsOption] as bool,
serveTargets: serveTargets,
assumeTty: argResults[assumeTtyOption] as bool,
Expand Down
5 changes: 2 additions & 3 deletions build_runner/lib/src/entrypoint/serve.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ServeCommand extends WatchCommand {
defaultsTo: false,
negatable: false,
help: 'Enables logging for each request to the server.')
..addFlag(liveReloadOption,
..addFlag(hotReloadOption,
defaultsTo: false,
negatable: false,
help: 'Enables automatic page reloading on rebuilds.');
Expand Down Expand Up @@ -90,8 +90,7 @@ class ServeCommand extends WatchCommand {
serveRequests(
server,
handler.handlerFor(target.dir,
logRequests: options.logRequests,
liveReload: options.liveReload));
logRequests: options.logRequests, hotReload: options.hotReload));
});

_ensureBuildWebCompilersDependency(packageGraph, logger);
Expand Down
2 changes: 1 addition & 1 deletion build_runner/lib/src/server/hot_reload_client/client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import 'reload_handler.dart';
import 'reloading_manager.dart';

final _assetsDigestPath = r'$assetDigests';
final _buildUpdatesProtocol = r'$livereload';
final _buildUpdatesProtocol = r'$hotreload';

@anonymous
@JS()
Expand Down
Loading