Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 4 additions & 0 deletions build_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.10.1-dev

- Added `--live-reload` cli option and appropriate functionality

## 0.10.0

### Breaking Changes
Expand Down
1 change: 1 addition & 0 deletions build_runner/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Some commands also have additional options:
##### serve

- `--hostname`: The host to run the server on.
- `--live-reload`: Enables automatic page reloading on rebuilds.
Copy link
Member

Choose a reason for hiding this comment

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

[nit] Don't include an extra newline between bullet points.


Trailing args of the form `<directory>:<port>` are supported to customize what
directories are served, and on what ports.
Expand Down
66 changes: 50 additions & 16 deletions build_runner/lib/src/server/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ import 'path_to_asset_id.dart';

const _performancePath = r'$perf';
final _graphPath = r'$graph';
final _buildUpdatesPath = r'$livereload';
final _buildUpdatesProtocol = r'$livereload';
final _buildUpdatesMessage = 'update';
final _entrypointExtensionMarker = '/* ENTRYPOINT_EXTENTION_MARKER */';

final _logger = new Logger('Serve');

Expand Down Expand Up @@ -85,12 +87,14 @@ class ServeHandler implements BuildState {
var assetHandler = await _assetHandler;
return assetHandler.handle(request, rootDir);
});
var handler = logRequests
? const shelf.Pipeline()
.addMiddleware(_logRequests)
.addHandler(cascade.handler)
: cascade.handler;
return handler;
var pipeline = shelf.Pipeline();
if (logRequests) {
pipeline = pipeline.addMiddleware(_logRequests);
}
if (liveReload) {
pipeline = pipeline.addMiddleware(_injectBuildUpdatesClientCode);
}
return pipeline.addHandler(cascade.handler);
}

Future<shelf.Response> _blockOnCurrentBuild(_) async {
Expand Down Expand Up @@ -119,34 +123,64 @@ class ServeHandler implements BuildState {
}
}

/// Class that manages web socket connection handler to inform clients about
/// build updates
class BuildUpdatesWebSocketHandler {
Copy link
Member

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>[];
shelf.Handler _internalHandler;

BuildUpdatesWebSocketHandler() {
_internalHandler = webSocketHandler(_handleConnection);
_internalHandler = webSocketHandler(_handleConnection, protocols: [_buildUpdatesProtocol]);
}

Future<shelf.Response> handler(shelf.Request request) async {
if (!request.url.path.startsWith(_buildUpdatesPath)) {
return new shelf.Response.notFound('');
}
return _internalHandler(request);
}
shelf.Handler get handler => _internalHandler;

void emitUpdateMessage(BuildResult buildResult) {
for (var webSocket in activeConnections) {
webSocket.sink.add('update');
webSocket.sink.add(_buildUpdatesMessage);
}
}

void _handleConnection(WebSocketChannel webSocket) async {
void _handleConnection(WebSocketChannel webSocket, String protocol) async {
activeConnections.add(webSocket);
await webSocket.stream.drain();
activeConnections.remove(webSocket);
}
}

shelf.Handler _injectBuildUpdatesClientCode(shelf.Handler innerHandler) {
return (shelf.Request request) {
if (!request.url.path.endsWith('.js')) {
return innerHandler(request);
}
return Future.sync(() => innerHandler(request)).then((response) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that you can await a FutureOr (or any synchronous value actually), which could clean this us just a bit.

// TODO: Find a way how to check and/or modify body without reading it whole
Copy link
Member

Choose a reason for hiding this comment

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

This is probably ok since most .js is going to be UTF8 encoded. We could also consider implementing a StreamTransformer that would do this but it's probably more complex that is warranted.

var body = await response.readAsString();
if (body.startsWith(_entrypointExtensionMarker)) {
body += _buildUpdatesInjectedJS;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should replace the placeholder comment as opposed to appending

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 don't think so.

  1. Why do not remove comment - we might have other middlewares in future that will depend on the same extension point
  2. Why append to the end of file - I feel it "safer" to append code to the end then to the begining

Copy link
Member

Choose a reason for hiding this comment

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

What is the browser behavior if there is some valid JS source followed by invalid stuff - will the stuff at the beginning of the file still get executed?
In our particular setup we're pretty sure we're always sending valid parseable javascript, but if the browser would start running our injected stuff even if the rest of the file contains broken stuff that could be a reason to put it at the beginning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICR if there is fatal syntax error - it won't. if "invalid stuff" can somehow be parsed as JS, then file will be executed till the first unhanded error

}
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
Copy link
Member

Choose a reason for hiding this comment

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

[nit] typo

final _buildUpdatesInjectedJS = '''
(function() {
Copy link
Contributor

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) {
console.log(event);
if(event.data === '$_buildUpdatesMessage'){
location.reload();
}
};
}());
''';

class AssetHandler {
final FinalizedReader _reader;
final String _rootPackage;
Expand Down
2 changes: 1 addition & 1 deletion build_runner/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build_runner
version: 0.10.0
version: 0.10.1-dev
description: Tools to write binaries that run builders.
author: Dart Team <[email protected]>
homepage: https://github.com/dart-lang/build/tree/master/build_runner
Expand Down
35 changes: 5 additions & 30 deletions build_web_compilers/lib/src/dev_compiler_bootstrap.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,9 @@ Future<Null> bootstrapDdc(BuildStep buildStep,
: _context.joinAll(_context.split(jsId.path).skip(1)));
}

var bootstrapContent = new StringBuffer('(function() {\n');
var bootstrapContent = new StringBuffer('$_entrypointExtensionMarker\n(function() {\n');
bootstrapContent.write(_dartLoaderSetup(modulePaths));
bootstrapContent.write(_requireJsConfig);
bootstrapContent.write(_hotReloadConfig);

bootstrapContent.write(_appBootstrap(
appModuleName, appModuleScope, ignoreCastFailures, enableSyncAsync));
Expand Down Expand Up @@ -322,35 +321,11 @@ require.config({
});
''';

/// Hot-reload config
/// Marker comment used by build_runner (or any other thinks) to identify
/// entrypoint file, to inject custom code there.
///
/// Listen WebSocket for updates in build results
///
/// Now only ilve-reload functional - just reload page on update message
final _hotReloadConfig = '''
(function() {
var wsUrl;
if (baseUrl.startsWith('http')) {
wsUrl = baseUrl.replace('http', 'ws');
} else {
// TODO: when it may make sense? WebWorkers? Tests?
wsUrl = 'ws://' + location.host + baseUrl;
}
wsUrl += '\$livereload';
try {
var ws = new WebSocket(wsUrl);
ws.onmessage = function(event) {
console.log(event);
if(event.data === 'update'){
location.reload();
}
};
} catch (e) {
// Live reload might be turned off. Just ignore it for now
// TODO: Find a way to pass serve option here, to not inject this code at all, if live/hot reload is turned off
}
}());
''';
/// Should be first line in a file, so server don't need to parse whole body
final _entrypointExtensionMarker = '/* ENTRYPOINT_EXTENTION_MARKER */';

final _baseUrlScript = '''
var baseUrl = (function () {
Expand Down