Skip to content
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.

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

Fixes #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
33 changes: 24 additions & 9 deletions lib/src/jaguar_hotreload_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@
// is governed by a BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:developer' as dev;
import 'dart:io';
import 'dart:isolate';
import 'package:glob/glob.dart';
import 'package:vm_service_lib/vm_service_lib_io.dart' show vmServiceConnectUri;
import 'package:vm_service_lib/vm_service_lib.dart' show VmService;
import 'package:vm_service/vm_service_io.dart' show vmServiceConnectUri;
import 'package:vm_service/vm_service.dart' show VmService;
import 'package:watcher/watcher.dart';

const String _kVmServiceUrl = 'ws://localhost:8181/ws';

/// Encapsulates Hot reloader path
class HotReloaderPath {
/// [HotReloader] the path belongs to
Expand Down Expand Up @@ -67,7 +66,7 @@ class HotReloader {
///
/// More information can be found at:
/// https://www.dartlang.org/dart-vm/tools/dart-vm
final String vmServiceUrl;
final FutureOr<String> vmServiceUrl;

/// Debounce interval for [onChange] event
final Duration debounceInterval;
Expand Down Expand Up @@ -107,10 +106,11 @@ class HotReloader {

/// Creates a [HotReloader] with given [vmServiceUrl]
///
/// By default, [vmServiceUrl] uses `ws://localhost:8181/ws`
/// By default, [vmServiceUrl] will be auto-detected locally.
HotReloader(
{this.vmServiceUrl = _kVmServiceUrl,
this.debounceInterval = const Duration(seconds: 1)}) {
{FutureOr<String> vmServiceUrl,
this.debounceInterval = const Duration(seconds: 1)})
: this.vmServiceUrl = vmServiceUrl ?? _detectServiceUrl() {
if (!isHotReloadable) throw notHotReloadable;

_onChangeSub = onChange
Expand Down Expand Up @@ -347,7 +347,7 @@ More information can be found at: https://www.dartlang.org/dart-vm/tools/dart-vm
print('Reloading the application...');

// Get vm service
_client ??= await vmServiceConnectUri(vmServiceUrl);
_client ??= await vmServiceConnectUri(await vmServiceUrl);

// Find main isolate id to reload it
final vm = await _client.getVM();
Expand Down Expand Up @@ -397,3 +397,18 @@ class _FoldedDebounce
});
}
}

Future<String> _detectServiceUrl() async {
var info = await dev.Service.getInfo();
var uri = info.serverUri;

uri = uri.replace(path: (uri.path.endsWith('/') ? uri.path : uri.path + '/') + 'ws');

if (uri.scheme == 'https') {
uri = uri.replace(scheme: 'wss');
} else {
uri = uri.replace(scheme: 'ws');
}

return uri.toString();
}
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ environment:
sdk: '>=2.0.0 <3.0.0'

dependencies:
vm_service_lib: ^0.3.5
vm_service: ^2.1.1
watcher: ^0.9.7
glob: ^1.1.3

Choose a reason for hiding this comment

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

Maybe take the opportunity to upgrade to latest version (1.2.0)?

Copy link
Author

Choose a reason for hiding this comment

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

Latest version of what?

Choose a reason for hiding this comment

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

This line above :)

glob: ^1.1.3

Line 17, where this comment is!

Copy link
Author

Choose a reason for hiding this comment

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

My screen shows an except of 6 lines, nothing highlighting line 17 specifically - so I think maybe GitHub has a bit of a UI problem there. 😉

Anyhow, the changes I made do not in any way relate to the use of glob - the package is still compatible with the 1.1 series of this package, as no new features from the 1.2 series are required, so, no.

You should let the consumer and/or package manager decide which version they need/want - this is the problem that package managers were intended to solve. The system works. 😄


Expand Down