Skip to content

Add simple live-reload implementation #1703

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 13 commits into from
Aug 4, 2018
Merged

Add simple live-reload implementation #1703

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0c3d126
Add live-reload option
samogot Aug 1, 2018
867252e
Add server-side WebSocket handler to inform clients about build state
samogot Aug 1, 2018
c18faa2
Implement simple live reloading on client side
samogot Aug 1, 2018
abe6af9
Reformat changes
samogot Aug 2, 2018
60890f4
Fix review comments
samogot Aug 2, 2018
f114ee5
Update pubspec.yaml, CHANGELOG.md and README.md
samogot Aug 2, 2018
12ceef4
Merge remote-tracking branch 'origin/master' into live-reload
samogot Aug 2, 2018
716ddcd
Move WebSocket client injection from build_web_compilers to build_runner
samogot Aug 2, 2018
03569c5
Add tests
samogot Aug 2, 2018
7374c99
Add more tests and reformat
samogot Aug 3, 2018
6a581ec
Improve tests
samogot Aug 3, 2018
86ecfc8
Last nit comments
samogot Aug 3, 2018
3c03e19
Update the version of build_web_compilers
samogot Aug 3, 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
29 changes: 16 additions & 13 deletions build_runner/lib/src/server/server.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,14 @@ class ServeHandler implements BuildState {
/// build updates
class BuildUpdatesWebSocketHandler {
Copy link
Member

@natebosch natebosch Aug 2, 2018

Choose a reason for hiding this comment

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

a Doc comment would be good here

final activeConnections = <WebSocketChannel>[];
final Function _handlerFactory;
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

lets type this more strongly if possible, something like

Handler Function(void Function(WebSocket, String), {List<String> protocols})

A bit verbose to be sure, but nicely typed :)

shelf.Handler _internalHandler;

BuildUpdatesWebSocketHandler() {
_internalHandler = webSocketHandler(_handleConnection, protocols: [_buildUpdatesProtocol]);
BuildUpdatesWebSocketHandler([this._handlerFactory = webSocketHandler]) {
var untypedTearOff = (webSocket, protocol) =>
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

one last nit, can you document why we need to do this, and ideally point at an issue in the original package?

Copy link
Contributor Author

@samogot samogot Aug 3, 2018

Choose a reason for hiding this comment

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

Created dart-archive/shelf_web_socket#11

_handleConnection(webSocket as WebSocketChannel, protocol as String);
_internalHandler = _handlerFactory(
untypedTearOff, protocols: [_buildUpdatesProtocol]) as shelf.Handler;
}

shelf.Handler get handler => _internalHandler;
Expand All @@ -149,27 +153,26 @@ class BuildUpdatesWebSocketHandler {
}

shelf.Handler _injectBuildUpdatesClientCode(shelf.Handler innerHandler) {
return (shelf.Request request) {
return (shelf.Request request) async {
if (!request.url.path.endsWith('.js')) {
return innerHandler(request);
}
return Future.sync(() => innerHandler(request)).then((response) async {
// TODO: Find a way how to check and/or modify body without reading it whole
var body = await response.readAsString();
if (body.startsWith(_entrypointExtensionMarker)) {
body += _buildUpdatesInjectedJS;
}
return response.change(body: body);
});
var response = await innerHandler(request);
// TODO: Find a way how to check and/or modify body without reading it whole
var body = await response.readAsString();
if (body.startsWith(_entrypointExtensionMarker)) {
body += _buildUpdatesInjectedJS;
}
return response.change(body: body);
};
}

/// Hot-reload config
///
/// Listen WebSocket for updates in build results
///
/// Now only ilve-reload functional - just reload page on update message
final _buildUpdatesInjectedJS = '''
/// Now only live-reload functional - just reload page on update message
final _buildUpdatesInjectedJS = '''\n
(function() {
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

Oh also its probably worth adding a comment here // Injected by build_runner for live reload support or something along those lines

var ws = new WebSocket('ws://' + location.host, ['$_buildUpdatesProtocol']);
ws.onmessage = function(event) {
Expand Down
93 changes: 93 additions & 0 deletions build_runner/test/server/serve_handler_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:io';

import 'package:logging/logging.dart';
import 'package:shelf/shelf.dart';
import 'package:stream_channel/stream_channel.dart';
import 'package:test/test.dart';

import 'package:build_runner/build_runner.dart';
Expand All @@ -20,6 +21,7 @@ import 'package:build_runner/src/server/server.dart';

import 'package:_test_common/common.dart';
import 'package:_test_common/package_graphs.dart';
import 'package:web_socket_channel/web_socket_channel.dart';

void main() {
ServeHandler serveHandler;
Expand Down Expand Up @@ -162,6 +164,97 @@ void main() {
expect(await response.readAsString(), contains('--track-performance'));
});
});

group('build updates', () {
final _entrypointExtensionMarker = '/* ENTRYPOINT_EXTENTION_MARKER */';
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

nit: you can import from the src directory and use the actual original definition of this (its fine to make it public there as long as it isn't exported by lib/build_runner.dart). Or, we could intentionally expose it so that build_web_compilers can use it.


test('injects client code if enabled', () async {
_addSource('a|web/some.js', _entrypointExtensionMarker);
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

nit: append some other js content to the end as well

var response = await serveHandler.handlerFor('web', liveReload: true)(
new Request('GET', Uri.parse('http://server.com/some.js')));
expect(await response.readAsString(), contains('\$livereload'));
});

test('doesn\'t inject client code if disabled', () async {
_addSource('a|web/some.js', _entrypointExtensionMarker);
var response = await serveHandler.handlerFor('web', liveReload: false)(
new Request('GET', Uri.parse('http://server.com/some.js')));
expect(await response.readAsString(), isNot(contains('\$livereload')));
});

test('doesn\'t inject client code in non-js files', () async {
_addSource('a|web/some.html', _entrypointExtensionMarker);
var response = await serveHandler.handlerFor('web', liveReload: true)(
new Request('GET', Uri.parse('http://server.com/some.html')));
expect(await response.readAsString(), isNot(contains('\$livereload')));
});

test('doesn\'t inject client code in non-marked files', () async {
_addSource('a|web/some.js', 'content');
var response = await serveHandler.handlerFor('web', liveReload: true)(
new Request('GET', Uri.parse('http://server.com/some.js')));
expect(await response.readAsString(), isNot(contains('\$livereload')));
});

test('emmits a message to all listners', () async {
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

nit: this test and the next one share a lot of setup logic, you can move them into a group (see 2nd code snippet here).

That lets you define a shared setUp and tearDown function for both tests.

Function exposedOnConnect;
var mockHandlerFactory = (Function onConnect, {protocols}) {
exposedOnConnect = onConnect;
};
var buildUpdatesWebSocketHandler = BuildUpdatesWebSocketHandler(
mockHandlerFactory);

var streamControllerFirst1 = StreamController<List<int>>();
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

nit: can we name these a bit more descriptively?

It is also important to make sure these get closed (in a tearDown method ideally).

var streamControllerFirst2 = StreamController<List<int>>();
var serverChannelFirst = WebSocketChannel(StreamChannel(streamControllerFirst1.stream, streamControllerFirst2.sink), serverSide: true);
var clientChannelFirst = WebSocketChannel(StreamChannel(streamControllerFirst2.stream, streamControllerFirst1.sink), serverSide: false);

var streamControllerSecond1 = StreamController<List<int>>();
var streamControllerSecond2 = StreamController<List<int>>();
var serverChannelSecond = WebSocketChannel(StreamChannel(streamControllerSecond1.stream, streamControllerSecond2.sink), serverSide: true);
var clientChannelSecond = WebSocketChannel(StreamChannel(streamControllerSecond2.stream, streamControllerSecond1.sink), serverSide: false);

var deferredExpect1 = clientChannelFirst.stream.single.then((value) => expect(value, 'update'));
var deferredExpect2 = clientChannelSecond.stream.single.then((value) => expect(value, 'update'));
exposedOnConnect(serverChannelFirst, '');
exposedOnConnect(serverChannelSecond, '');
buildUpdatesWebSocketHandler.emitUpdateMessage(null);
await clientChannelFirst.sink.close();
await clientChannelSecond.sink.close();
await deferredExpect1;
await deferredExpect2;
});

test('deletes listners on disconect', () async {
Function exposedOnConnect;
var mockHandlerFactory = (Function onConnect, {protocols}) {
exposedOnConnect = onConnect;
};
var buildUpdatesWebSocketHandler = BuildUpdatesWebSocketHandler(
mockHandlerFactory);

var streamControllerFirst1 = StreamController<List<int>>();
var streamControllerFirst2 = StreamController<List<int>>();
var serverChannelFirst = WebSocketChannel(StreamChannel(streamControllerFirst1.stream, streamControllerFirst2.sink), serverSide: true);
var clientChannelFirst = WebSocketChannel(StreamChannel(streamControllerFirst2.stream, streamControllerFirst1.sink), serverSide: false);

var streamControllerSecond1 = StreamController<List<int>>();
var streamControllerSecond2 = StreamController<List<int>>();
var serverChannelSecond = WebSocketChannel(StreamChannel(streamControllerSecond1.stream, streamControllerSecond2.sink), serverSide: true);
var clientChannelSecond = WebSocketChannel(StreamChannel(streamControllerSecond2.stream, streamControllerSecond1.sink), serverSide: false);

var deferredExpect1 = clientChannelFirst.stream.toList().then((value) => expect(value, ['update', 'update']));
Copy link
Contributor

@jakemac53 jakemac53 Aug 3, 2018

Choose a reason for hiding this comment

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

Instead of this you can use the built in stream matchers

var deferredExpect2 = clientChannelSecond.stream.single.then((value) => expect(value, 'update'));
exposedOnConnect(serverChannelFirst, '');
exposedOnConnect(serverChannelSecond, '');
buildUpdatesWebSocketHandler.emitUpdateMessage(null);
await clientChannelSecond.sink.close();
buildUpdatesWebSocketHandler.emitUpdateMessage(null);
await clientChannelFirst.sink.close();
await deferredExpect1;
await deferredExpect2;
});
});
}

class MockWatchImpl implements WatchImpl {
Expand Down