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

Modify DevToolsExtensionConfig model to handle static extensions #7602

Merged
merged 21 commits into from
Apr 17, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Do not delete these arguments. They are parsed by test runner.
// test-argument:experimentsOn=true

import 'package:devtools_app/devtools_app.dart';
import 'package:devtools_app/src/extensions/embedded/view.dart';
import 'package:devtools_app/src/extensions/extension_screen.dart';
Expand Down
24 changes: 16 additions & 8 deletions packages/devtools_app/lib/src/shared/development_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,9 @@ final List<DevToolsExtensionConfig> debugExtensions = [
DevToolsExtensionConfig.versionKey: '1.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: '0xe0b1',
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/foo',
DevToolsExtensionConfig.devtoolsOptionsUriKey: '/path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
}),
DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/bar',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'false',
}),
DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
Expand All @@ -105,7 +99,21 @@ final List<DevToolsExtensionConfig> debugExtensions = [
DevToolsExtensionConfig.versionKey: '3.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe50a,
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/provider',
DevToolsExtensionConfig.devtoolsOptionsUriKey: '/path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'true',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'false',
}),
// Static extension.
DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey: '/path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
}),
];

Expand Down
3 changes: 1 addition & 2 deletions packages/devtools_app/lib/src/shared/server/_dtd_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ part of 'server.dart';

/// Asks the Devtools Server to return a Dart Tooling Daemon uri if it has one.
Future<Uri?> getDtdUri() async {
if (debugDtdUri != null) return Uri.parse(debugDtdUri!);
if (isDevToolsServerAvailable) {
final uri = Uri(path: DtdApi.apiGetDtdUri);
final resp = await request(uri.toString());
Expand All @@ -14,8 +15,6 @@ Future<Uri?> getDtdUri() async {
final uriString = parsedResult[DtdApi.uriPropertyName] as String?;
return uriString != null ? Uri.parse(uriString) : null;
}
} else if (debugDtdUri != null) {
return Uri.parse(debugDtdUri!);
}
return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ part of 'server.dart';
Future<List<DevToolsExtensionConfig>> refreshAvailableExtensions(
Uri appRoot,
) async {
_log.fine('refreshAvailableExtensions for ${appRoot.toString()}');
_log.fine('refreshAvailableExtensions for app root: ${appRoot.toString()}');
if (debugDevToolsExtensions) {
return debugHandleRefreshAvailableExtensions();
}
if (isDevToolsServerAvailable) {
final uri = Uri(
path: ExtensionsApi.apiServeAvailableExtensions,
Expand Down Expand Up @@ -49,8 +52,6 @@ Future<List<DevToolsExtensionConfig>> refreshAvailableExtensions(
logWarning(resp, ExtensionsApi.apiServeAvailableExtensions);
return [];
}
} else if (debugDevToolsExtensions) {
return debugHandleRefreshAvailableExtensions();
}
return [];
}
Expand All @@ -70,6 +71,12 @@ Future<ExtensionEnabledState> extensionEnabledState({
_log.fine(
'${enable != null ? 'setting' : 'getting'} extensionEnabledState for $extensionName',
);
if (debugDevToolsExtensions) {
return debugHandleExtensionEnabledState(
extensionName: extensionName,
enable: enable,
);
}
if (isDevToolsServerAvailable) {
final uri = Uri(
path: ExtensionsApi.apiExtensionEnabledState,
Expand All @@ -89,11 +96,6 @@ Future<ExtensionEnabledState> extensionEnabledState({
} else {
logWarning(resp, ExtensionsApi.apiExtensionEnabledState);
}
} else if (debugDevToolsExtensions) {
return debugHandleExtensionEnabledState(
extensionName: extensionName,
enable: enable,
);
}
return ExtensionEnabledState.error;
}
35 changes: 5 additions & 30 deletions packages/devtools_app/test/test_infra/test_data/extensions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_shared/src/extensions/extension_model.dart';
import 'package:devtools_app/src/shared/development_helpers.dart';

final testExtensions = [fooExtension, barExtension, providerExtension];

final fooExtension = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'foo',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '1.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: '0xe0b1',
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/foo',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
});

final barExtension = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/bar',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
});

final providerExtension = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
DevToolsExtensionConfig.issueTrackerKey:
'https://github.com/rrousselGit/provider/issues',
DevToolsExtensionConfig.versionKey: '3.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe50a,
DevToolsExtensionConfig.extensionAssetsUriKey: '/path/to/provider',
DevToolsExtensionConfig.isPubliclyHostedKey: 'true',
});
final testExtensions = List.of(debugExtensions)..sort();
final barExtension = testExtensions[0];
final fooExtension = testExtensions[1];
final providerExtension = testExtensions[2];
1 change: 1 addition & 0 deletions packages/devtools_extensions/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## 0.2.0-dev.0
* Update `extension_config_spec.md` to include an optional field `requiresConnection`.
* Bump `devtools_shared` dependency to `^10.0.0`.
* Fix file locations in the `dart_foo` extension example.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ name: dart_foo
issueTracker: https://www.google.com/
version: 1.0.0
materialIconCodePoint: "0xe50a"
requiresConnection: false
16 changes: 15 additions & 1 deletion packages/devtools_extensions/extension_config_spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,25 @@ title bar.
[material/icons.dart](https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/material/icons.dart).
This icon will be used for the extension’s tab in the top-level DevTools tab bar.

## Example
## Optional fields
- `requiresConnection`: whether this DevTools extension requires a connected Dart or
Flutter application to run. If this is not specified, this value will default to `true`.

## Examples

An extension for `foo_package` that requires a connected app to use:
```yaml
name: foo_package
issueTracker: <link_to_your_issue_tracker.com>
version: 0.0.1
materialIconCodePoint: '0xe0b1'
```

An extension for `foo_package` that does not require a connected app to use:
```yaml
name: foo_package
issueTracker: <link_to_your_issue_tracker.com>
version: 0.0.1
materialIconCodePoint: '0xe0b1'
requiresConnection: false
```
7 changes: 6 additions & 1 deletion packages/devtools_shared/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
* **Breaking change:** rename `ExtensionsApi.extensionRootPathPropertyName`
to `ExtensionsApi.packageRootUriPropertyName`, and modify the String value
for the parameter from 'rootPath' to 'packageRootUri'.

* **Breaking change:** add new required JSON fields "devtoolsOptionsUri" and
Copy link
Contributor

Choose a reason for hiding this comment

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

How are we planning on handling these breaking changes for existing extensions in the ecosystem?

Copy link
Member Author

Choose a reason for hiding this comment

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

These breaking changes are for the DevTools --> DevTools server API and shouldn't be called directly by DevTools extensions. The devtools_extensions and devtools_app_shared packages will be bumped to this new devtools_shared version in their own major version bumps.

"detectedFromStaticContext" in the `DevToolsExtensionConfig.parse` factory constructor.
* **Breaking change:** remove `DevToolsOptions.optionsFileName` constant in favor of
new constant `devtoolsOptionsFileName`.
* Add new fields `requiresConnection`, `devtoolsOptionsUri`, and `detectedFromStaticContext`
to `DevToolsExtensionConfig`.
* Return valid extensions from the `apiServeAvailableExtensions` endpoint even when
an exception is thrown.
* Add utility extension methods on `Completer`: `safeComplete` and `safeCompleteError`.
Expand Down
4 changes: 4 additions & 0 deletions packages/devtools_shared/lib/src/extensions/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@
/// Location where DevTools extension assets will be served, relative to where
/// DevTools assets are served (build/).
const extensionRequestPath = 'devtools_extensions';

/// The name of the options file where extension enablement states are stored
/// in a user's project.
const devtoolsOptionsFileName = 'devtools_options.yaml';
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import 'package:path/path.dart' as path;
import 'package:yaml/yaml.dart';
import 'package:yaml_edit/yaml_edit.dart';

import 'constants.dart';
import 'extension_model.dart';
import 'yaml_utils.dart';

/// Manages the `devtools_options.yaml` file and allows read / write access.
class DevToolsOptions {
static const optionsFileName = 'devtools_options.yaml';
static const _extensionsKey = 'extensions';
static const _descriptionKey = 'description';
static const _documentationKey = 'documentation';
Expand Down Expand Up @@ -128,7 +128,7 @@ $_extensionsKey:
return null;
}

final optionsFile = File(path.join(rootDir.path, optionsFileName));
final optionsFile = File(path.join(rootDir.path, devtoolsOptionsFileName));
if (!optionsFile.existsSync()) {
optionsFile
..createSync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:collection/collection.dart';
import 'package:extension_discovery/extension_discovery.dart';
import 'package:path/path.dart' as path;

import 'constants.dart';
import 'extension_model.dart';

/// The default location for the DevTools extension, relative to
Expand Down Expand Up @@ -57,28 +58,22 @@ class ExtensionsManager {
String? rootFileUriString,
List<String> logs,
) async {
if (rootFileUriString != null && !rootFileUriString.startsWith('file://')) {
throw ArgumentError.value(
rootFileUriString,
'rootPathFileUri',
'must be a file:// URI String',
);
}

logs.add(
'ExtensionsManager.serveAvailableExtensions: '
'ExtensionsManager.serveAvailableExtensions for '
'rootPathFileUri: $rootFileUriString',
);

_clear();
final parsingErrors = StringBuffer();

// Find all runtime extensions for [rootFileUriString], if non-null.
if (rootFileUriString != null) {
// Find all runtime extensions for [rootFileUriString], if non-null and
// non-empty.
if (rootFileUriString != null && rootFileUriString.isNotEmpty) {
await _addExtensionsForRoot(
rootFileUriString,
logs: logs,
parsingErrors: parsingErrors,
staticContext: false,
);
}

Expand All @@ -97,21 +92,26 @@ class ExtensionsManager {
String rootFileUriString, {
required List<String> logs,
required StringBuffer parsingErrors,
required bool staticContext,
}) async {
_assertUriFormat(rootFileUriString);
late final List<Extension> extensions;
try {
// TODO(https://github.com/dart-lang/pub/issues/4218): this assumes that
// the .dart_tool/package_config.json file is in the package root, which
// may be an incorrect assumption for monorepos.
final packageConfigPath = path.posix.join(
rootFileUriString,
'.dart_tool',
'package_config.json',
);
extensions = await findExtensions(
'devtools',
packageConfig: Uri.parse(
path.posix.join(
rootFileUriString,
'.dart_tool',
'package_config.json',
),
),
packageConfig: Uri.parse(packageConfigPath),
);
logs.add(
'ExtensionsManager.serveAvailableExtensions: findExtensionsResult - '
'ExtensionsManager._addExtensionsForRoot find extensions for '
'config: $packageConfigPath, result: '
'${extensions.map((e) => e.package).toList()}',
);
} catch (e) {
Expand Down Expand Up @@ -143,7 +143,14 @@ class ExtensionsManager {
final extensionConfig = DevToolsExtensionConfig.parse({
...config,
DevToolsExtensionConfig.extensionAssetsUriKey: location,
// TODO(kenz): for monorepos, we may want to store the
// devtools_options.yaml at the same location as the workspace's
// .dart_tool/package_config.json file.
DevToolsExtensionConfig.devtoolsOptionsUriKey:
path.join(rootFileUriString, devtoolsOptionsFileName),
DevToolsExtensionConfig.isPubliclyHostedKey: isPubliclyHosted,
DevToolsExtensionConfig.detectedFromStaticContextKey:
staticContext.toString(),
});
devtoolsExtensions.add(extensionConfig);
} on StateError catch (e) {
Expand All @@ -153,6 +160,12 @@ class ExtensionsManager {
}
}

void _assertUriFormat(String? uriString) {
if (uriString != null && !uriString.startsWith('file://')) {
throw ArgumentError.value(uriString, 'must be a file:// URI String');
}
}

void _clear() {
_extensionLocationsByIdentifier.clear();
devtoolsExtensions.clear();
Expand Down
Loading
Loading