Skip to content
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
16 changes: 11 additions & 5 deletions packages/devtools_app/lib/src/extensions/extension_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ class ExtensionService extends DisposableController
// not always be true for extensions that are not published on pub or
// extensions that do not follow best practices for naming.
final isRuntimeDuplicate = runtimeExtensions.any(
(ext) => ext.name == staticExtension.name,
(ext) =>
ext.packageName == staticExtension.packageName &&
ext.name == staticExtension.name,
);
if (isRuntimeDuplicate) {
_log.fine(
Expand All @@ -256,6 +258,7 @@ class ExtensionService extends DisposableController
final stateFromOptionsFile = await server.extensionEnabledState(
devtoolsOptionsFileUri: extension.devtoolsOptionsUri,
extensionName: extension.name,
extensionPackage: extension.packageName,
);
final stateNotifier = _extensionEnabledStates.putIfAbsent(
extension.name,
Expand Down Expand Up @@ -292,15 +295,18 @@ class ExtensionService extends DisposableController
// Set the enabled state for all matching extensions, even if some are
// marked as ignored due to being a duplicate. This ensures that
// devtools_options.yaml files are kept in sync across the project.
final allMatchingExtensions = [
...runtimeExtensions,
...staticExtensions,
].where((e) => e.name == extension.name);
final allMatchingExtensions = [...runtimeExtensions, ...staticExtensions]
.where(
(e) =>
e.packageName == extension.packageName &&
e.name == extension.name,
);
await [
for (final ext in allMatchingExtensions)
server.extensionEnabledState(
devtoolsOptionsFileUri: ext.devtoolsOptionsUri,
extensionName: ext.name,
extensionPackage: ext.packageName,
enable: enable,
),
].wait;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ void deduplicateExtensionsAndTakeLatest(
}) {
final deduped = <String>{};
for (final ext in extensions) {
if (deduped.contains(ext.name)) continue;
deduped.add(ext.name);
final dedupKey = '${ext.packageName}:${ext.name}';
if (deduped.contains(dedupKey)) continue;
deduped.add(dedupKey);

// This includes [ext] itself.
final matchingExtensions = extensions.where((e) => e.name == ext.name);
final matchingExtensions = extensions.where(
(e) => e.packageName == ext.packageName && e.name == ext.name,
);
if (matchingExtensions.length > 1) {
logger?.fine(
'detected duplicate $extensionType extensions for ${ext.name}',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ Future<List<DevToolsExtensionConfig>> refreshAvailableExtensions(
Future<ExtensionEnabledState> extensionEnabledState({
required String devtoolsOptionsFileUri,
required String extensionName,
String? extensionPackage,
bool? enable,
}) async {
_log.fine(
'${enable != null ? 'setting' : 'getting'} extensionEnabledState for '
'$extensionName in options file ($devtoolsOptionsFileUri)',
'$extensionName (package: $extensionPackage) in options file ($devtoolsOptionsFileUri)',
);
if (debugDevToolsExtensions) {
return debugHandleExtensionEnabledState(
Expand All @@ -92,8 +93,8 @@ Future<ExtensionEnabledState> extensionEnabledState({
queryParameters: {
ExtensionsApi.devtoolsOptionsUriPropertyName: devtoolsOptionsFileUri,
ExtensionsApi.extensionNamePropertyName: extensionName,
if (enable != null)
ExtensionsApi.enabledStatePropertyName: enable.toString(),
ExtensionsApi.extensionPackagePropertyName: ?extensionPackage,
ExtensionsApi.enabledStatePropertyName: ?enable?.toString(),
Comment on lines +96 to +97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

[MUST-FIX] The syntax '?expression' is invalid in Dart map literals and will cause a compilation error. Use conditional elements ('if (condition)') to conditionally include these query parameters, matching the pattern used previously.

        if (extensionPackage != null)
          ExtensionsApi.extensionPackagePropertyName: extensionPackage,
        if (enable != null)
          ExtensionsApi.enabledStatePropertyName: enable.toString(),
References
  1. Prefix every comment with a severity: [MUST-FIX] for logical bugs, [CONCERN] for maintainability issues. (link)

},
);
final resp = await request(uri.toString());
Expand Down
3 changes: 3 additions & 0 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ TODO: Remove this section if there are not any updates.

* Hide the DevTools extensions menu button in single-screen embedded mode (`EmbedMode.embedOne`) on standard screens.
[#8507](https://github.com/flutter/devtools/issues/8507)
* Improved DevTools extension isolation by tracking the providing package name for
enablement, deduplication, and asset loading.
[#9965](https://github.com/flutter/devtools/pull/9965)

## Advanced developer mode updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,94 @@ void main() {
expect(takeLatestExtension(a, b), a);
});
});

group('deduplicateExtensionsAndTakeLatest', () {
test('deduplicates matching packageName and name', () {
final ignored = <DevToolsExtensionConfig>{};
final ext1 = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
DevToolsExtensionConfig.packageNameKey: 'provider',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '1.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/path/to/provider_1',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
final ext2 = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
DevToolsExtensionConfig.packageNameKey: 'provider',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/path/to/provider_2',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});

deduplicateExtensionsAndTakeLatest(
[ext1, ext2],
onSetIgnored: (ext, {required ignore}) {
if (ignore) {
ignored.add(ext);
} else {
ignored.remove(ext);
}
},
);

expect(ignored, contains(ext1));
expect(ignored, isNot(contains(ext2)));
});

test('does not deduplicate across different packageNames', () {
final ignored = <DevToolsExtensionConfig>{};
final providerExt = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
DevToolsExtensionConfig.packageNameKey: 'provider',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '1.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/path/to/provider',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
final spoofedExt = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'provider',
DevToolsExtensionConfig.packageNameKey: 'bad_pkg',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '999.0.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/path/to/bad_pkg',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});

deduplicateExtensionsAndTakeLatest(
[providerExt, spoofedExt],
onSetIgnored: (ext, {required ignore}) {
if (ignore) {
ignored.add(ext);
} else {
ignored.remove(ext);
}
},
);

// Neither should be ignored because they come from different packages.
expect(ignored, isEmpty);
});
});
}
1 change: 1 addition & 0 deletions packages/devtools_app_shared/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens
* Fix garbage collection issues with the result list in `asyncEval` on both native VM and web.
* The minimum Dart SDK version is bumped to 3.11.0.
* The minimum Flutter SDK version is bumped to 3.41.0.
* Updates `devtools_shared` constraint to `^14.0.1`.

## 0.5.1
* Add DevTools-styled text field `DevToolsTextField`.
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_app_shared/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ resolution: workspace
dependencies:
collection: ^1.15.0
dds_service_extensions: ^2.0.0
devtools_shared: ^14.0.0
devtools_shared: ^14.0.1
dtd: ^4.0.0
flutter:
sdk: flutter
Expand Down
1 change: 1 addition & 0 deletions packages/devtools_extensions/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ found in the LICENSE file or at https://developers.google.com/open-source/licens
## 0.5.2-wip
* The minimum Dart SDK version is bumped to 3.11.0.
* The minimum Flutter SDK version is bumped to 3.41.0.
* Updates `devtools_shared` constraint to `^14.0.1`.

## 0.5.1
* Updates `devtools_app_shared` constraint to `^0.5.1`.
Expand Down
19 changes: 19 additions & 0 deletions packages/devtools_extensions/bin/_validate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ void _validateDirectoryContents(String packagePath) {
throw FileSystemException('${packageDirectory.path} directory not found');
}

final pubspecFile = File(path.join(packageDirectory.path, 'pubspec.yaml'));
if (!pubspecFile.existsSync()) {
throw const FileSystemException('''
A pubspec.yaml file is required, but none was found.
See ${ValidateExtensionCommand.docUrl}.
''');
}

final devtoolsExtensionDir = Directory(
path.join(packageDirectory.path, 'extension', 'devtools'),
);
Expand Down Expand Up @@ -109,6 +117,17 @@ An extension/devtools/config.yaml file is required, but none was found.
See ${ValidateExtensionCommand.docUrl}.
''');
}

// Ensure the extension's name is a valid identifier.
final configYaml = _configAsMap(packagePath);
final configName = configYaml['name'] as String?;
final underscoresAndLetters = RegExp(r'^[a-z0-9_]*$');
if (configName == null || !underscoresAndLetters.hasMatch(configName)) {
Comment on lines +123 to +125

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

[CONCERN] Casting configYaml['name'] directly to String? using 'as String?' will throw a TypeError at runtime if the value in config.yaml is of another type (e.g., an integer or boolean). It is safer to perform a type check ('is! String') to throw a descriptive StateError instead.

Suggested change
final configName = configYaml['name'] as String?;
final underscoresAndLetters = RegExp(r'^[a-z0-9_]*$');
if (configName == null || !underscoresAndLetters.hasMatch(configName)) {
final configName = configYaml['name'];
final underscoresAndLetters = RegExp(r'^[a-z0-9_]*$');
if (configName is! String || !underscoresAndLetters.hasMatch(configName)) {
References
  1. Prefix every comment with a severity: [MUST-FIX] for logical bugs, [CONCERN] for maintainability issues. (link)

throw StateError(
'The "name" field in config.yaml should only contain lowercase letters, '
'numbers, and underscores but instead was "$configName".',
);
}
}

Map<String, Object?> _configAsMap(String packagePath) {
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_extensions/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ executables:

dependencies:
args: ^2.4.2
devtools_shared: ^14.0.0
devtools_shared: ^14.0.1
devtools_app_shared: ^0.5.1
flutter:
sdk: flutter
Expand Down
40 changes: 40 additions & 0 deletions packages/devtools_extensions/test/validate_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,44 @@ void main() {
});
}
});

group('devtools_extensions validate command fails', () {
test('when config.yaml name contains invalid characters', () async {
final tempDir = Directory.systemTemp.createTempSync();
try {
final extDir = Directory(p.join(tempDir.path, 'extension', 'devtools'))
..createSync(recursive: true);
Directory(p.join(extDir.path, 'build')).createSync(recursive: true);
File(p.join(extDir.path, 'build', 'index.html')).writeAsStringSync('');
File(p.join(extDir.path, 'config.yaml')).writeAsStringSync('''
name: invalid-name-with-hyphens
issueTracker: https://www.google.com/
version: 1.0.0
materialIconCodePoint: "0xe50a"
''');
File(p.join(tempDir.path, 'pubspec.yaml')).writeAsStringSync('''
name: actual_package_name
environment:
sdk: ^3.2.0
''');

final process = await Process.run('dart', [
'run',
'devtools_extensions',
'validate',
'-p',
tempDir.path,
]);
expect(
process.stderr,
contains(
'Validation error: The "name" field in config.yaml should only '
'contain lowercase letters, numbers, and underscores',
),
);
} finally {
tempDir.deleteSync(recursive: true);
}
});
});
}
6 changes: 5 additions & 1 deletion packages/devtools_shared/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
<!--
Copyright 2025 The Flutter Authors
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
-->
# 14.0.1

* Track providing package name for DevTools extensions to isolate extension enablement,
deduplication, and asset loading.

# 14.0.0

* **Breaking changes**: `LocalFileSystem`, an extension which provided some handy
Expand Down
5 changes: 5 additions & 0 deletions packages/devtools_shared/lib/src/devtools_api.dart
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ abstract class ExtensionsApi {
/// name of the extension whose state is being queried.
static const extensionNamePropertyName = 'name';

/// The property name for the query parameter optionally passed along with
/// [apiExtensionEnabledState] requests to the server that describes the
/// package name providing the extension.
static const extensionPackagePropertyName = 'package';

/// The property name for the query parameter that is optionally passed along
/// with [apiExtensionEnabledState] requests to the server to set the
/// enabled state for the extension.
Expand Down
Loading
Loading