[webdev] Update canary default setting in Configuration (#2870) The `canaryFeatures` flag now defaults to `true` to match the expected value for the `moduleFormat` default. New coercions based on the expected intent when setting other flags: * Passing `--web-hot-reload` coerces canary mode to `true`. * Setting 'amd' module format coerces canary mode to `false`. The existing tests were passing because they always set the module format and canary flags explicitly. The actual usage in VSCode saw the `canaryFeatures` flag default value in some code paths leading to inconsistent settings. Fixes: https://github.com/dart-lang/webdev/issues/2869
diff --git a/webdev/CHANGELOG.md b/webdev/CHANGELOG.md index fa314c8..fc4ec9d 100644 --- a/webdev/CHANGELOG.md +++ b/webdev/CHANGELOG.md
@@ -1,3 +1,7 @@ +## 4.0.3 + +- Fix failing debugger expression evaluation. + ## 4.0.2 - **Internal**: Resolve Chrome test flakes.
diff --git a/webdev/lib/src/command/configuration.dart b/webdev/lib/src/command/configuration.dart index a0023d2..219fe39 100644 --- a/webdev/lib/src/command/configuration.dart +++ b/webdev/lib/src/command/configuration.dart
@@ -317,8 +317,7 @@ String get moduleFormat => _moduleFormat ?? 'ddc'; - bool get usesDdcLibraryBundle => - canaryFeatures || (moduleFormat == 'ddc') || webHotReload; + bool get usesDdcLibraryBundle => canaryFeatures && moduleFormat == 'ddc'; bool get offline => _offline ?? false; @@ -450,42 +449,77 @@ ? argResults[webHotReloadFlag] as bool? : defaultConfiguration.webHotReload; - final canaryFeatures = + final canaryParsed = argResults.options.contains(canaryFeaturesFlag) && - argResults.wasParsed(canaryFeaturesFlag) - ? argResults[canaryFeaturesFlag] as bool? - : defaultConfiguration._canaryFeatures; + argResults.wasParsed(canaryFeaturesFlag); + + final canaryFeatures = + (canaryParsed + ? argResults[canaryFeaturesFlag] as bool? + : defaultConfiguration._canaryFeatures) + // Overwise match the default value of the CLI argument. + ?? + true; final moduleFormatParsed = argResults.options.contains(moduleFormatFlag) && argResults.wasParsed(moduleFormatFlag); - final moduleFormat = moduleFormatParsed - ? argResults[moduleFormatFlag] as String? - : (webHotReload == true ? 'ddc' : defaultConfiguration.moduleFormat); + final moduleFormat = + (moduleFormatParsed + ? argResults[moduleFormatFlag] as String? + : defaultConfiguration.moduleFormat) + // Overwise match the default value of the CLI argument. + ?? + 'ddc'; - if (webHotReload == true && !moduleFormatParsed) { - logWriter( - Level.INFO, - 'Coercing --$moduleFormatFlag to ddc because --$webHotReloadFlag is set.', - ); + // TODO(nshahan): Remove when DDC no longer requires canary + ddc modules + // for the library bundle format. + // https://github.com/dart-lang/webdev/issues/2871 + String? coercedModuleFormat; + bool? coercedCanaryFeatures; + if (webHotReload == true) { + // Always take enabling hot reload as the intent to override module format + // and canary mode since they are required for it to work. + if (moduleFormat != 'ddc') { + logWriter( + Level.INFO, + "Coercing --$moduleFormatFlag to 'ddc' " + 'because --$webHotReloadFlag is set.\n', + ); + coercedModuleFormat = 'ddc'; + } + if (!canaryFeatures) { + logWriter( + Level.INFO, + "Coercing --$canaryFeaturesFlag to 'true' " + 'because --$webHotReloadFlag is set.\n', + ); + coercedCanaryFeatures = true; + } + } else { + // Use the module format as a signal to validate or coerce the canary + // mode since that is the advertised way to revert to the old behavior. + if (moduleFormat == 'amd' && canaryFeatures) { + logWriter( + Level.INFO, + "Coercing --$canaryFeaturesFlag to 'false' " + "because --$moduleFormatFlag is set to 'amd'\n", + ); + coercedCanaryFeatures = false; + } else if (moduleFormat == 'ddc' && !canaryFeatures) { + logWriter( + Level.INFO, + "Coercing --$canaryFeaturesFlag to 'true' " + "because --$moduleFormatFlag is set to 'ddc'.\n", + ); + } } final offline = argResults.options.contains(offlineFlag) ? argResults[offlineFlag] as bool? : defaultConfiguration.offline; - final canaryParsed = - argResults.options.contains(canaryFeaturesFlag) && - argResults.wasParsed(canaryFeaturesFlag); - - if (webHotReload == true && !canaryParsed && canaryFeatures == null) { - logWriter( - Level.INFO, - 'Coercing --$canaryFeaturesFlag to true because --$webHotReloadFlag is set.', - ); - } - return Configuration( autoRun: defaultConfiguration.autoRun, chromeDebugPort: chromeDebugPort, @@ -510,8 +544,8 @@ verbose: verbose, experiments: experiments, webHotReload: webHotReload, - canaryFeatures: canaryFeatures, - moduleFormat: moduleFormat, + canaryFeatures: coercedCanaryFeatures ?? canaryFeatures, + moduleFormat: coercedModuleFormat ?? moduleFormat, offline: offline, ); }
diff --git a/webdev/lib/src/command/shared.dart b/webdev/lib/src/command/shared.dart index 12318ad..b492a84 100644 --- a/webdev/lib/src/command/shared.dart +++ b/webdev/lib/src/command/shared.dart
@@ -77,7 +77,7 @@ ..addFlag( canaryFeaturesFlag, abbr: 'c', - defaultsTo: false, + defaultsTo: true, negatable: true, hide: true, help: 'Enables DDC canary features.',
diff --git a/webdev/lib/src/version.dart b/webdev/lib/src/version.dart index 1d7ed1f..ee7a14f 100644 --- a/webdev/lib/src/version.dart +++ b/webdev/lib/src/version.dart
@@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '4.0.2'; +const packageVersion = '4.0.3';
diff --git a/webdev/pubspec.yaml b/webdev/pubspec.yaml index f8d1f6c..85a77bf 100644 --- a/webdev/pubspec.yaml +++ b/webdev/pubspec.yaml
@@ -1,6 +1,6 @@ name: webdev # Every time this changes you need to run `dart run build_runner build`. -version: 4.0.2 +version: 4.0.3 # We should not depend on a dev SDK before publishing. # publish_to: none description: >-
diff --git a/webdev/test/configuration_test.dart b/webdev/test/configuration_test.dart index a9d46c7..321bb13 100644 --- a/webdev/test/configuration_test.dart +++ b/webdev/test/configuration_test.dart
@@ -5,14 +5,15 @@ import 'package:args/args.dart'; import 'package:test/test.dart'; import 'package:webdev/src/command/configuration.dart'; +import 'package:webdev/src/command/shared.dart'; void main() { late ArgParser argParser; setUp(() { argParser = ArgParser() - ..addFlag('release') ..addFlag(launchInChromeFlag, defaultsTo: false) ..addOption(userDataDirFlag, defaultsTo: null); + addSharedArgs(argParser); }); test('default configuration is correctly applied', () { @@ -156,4 +157,20 @@ throwsA(isA<InvalidConfiguration>()), ); }); + + // TODO(nshahan): Remove when DDC no longer requires canary + ddc modules for + // the library bundle format. + // https://github.com/dart-lang/webdev/issues/2871 + test('canary is enabled by default for ddc module format', () { + final config = Configuration.fromArgs(argParser.parse([])); + expect(config.moduleFormat, equals('ddc')); + expect(config.canaryFeatures, isTrue); + }); + test('canary is coerced to false when --module-format=amd is passed', () { + final config = Configuration.fromArgs( + argParser.parse(['--module-format=amd']), + ); + expect(config.moduleFormat, equals('amd')); + expect(config.canaryFeatures, isFalse); + }); }
diff --git a/webdev/test/daemon/daemon_domain_common.dart b/webdev/test/daemon/daemon_domain_common.dart index 5b7fcdd..bd083c9 100644 --- a/webdev/test/daemon/daemon_domain_common.dart +++ b/webdev/test/daemon/daemon_domain_common.dart
@@ -30,7 +30,7 @@ ], workingDirectory: exampleDirectory); await expectLater( webdev.stdout, - emits(startsWith('[{"event":"daemon.connected"')), + emitsThrough(startsWith('[{"event":"daemon.connected"')), ); await exitWebdev(webdev); });