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

use the import information in the 'appIsFlutter' detection #3191

Open
wants to merge 1 commit into
base: main
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
4 changes: 2 additions & 2 deletions pkgs/dartpad_shared/lib/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ class SourceRequest {
class AnalysisResponse {
final List<AnalysisIssue> issues;

final List<String>? imports;
final List<String> imports;

AnalysisResponse({required this.issues, this.imports});
AnalysisResponse({required this.issues, required this.imports});

factory AnalysisResponse.fromJson(Map<String, Object?> json) =>
_$AnalysisResponseFromJson(json);
Expand Down
2 changes: 1 addition & 1 deletion pkgs/dartpad_shared/lib/model.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 16 additions & 15 deletions pkgs/dartpad_ui/lib/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,14 @@ abstract class EditorService {
}

class AppModel {
final ValueNotifier<bool?> _appIsFlutter = ValueNotifier(null);
bool? _usesPackageWeb;
final ValueNotifier<bool?> appIsFlutter = ValueNotifier(null);
AppType get appType =>
_appIsFlutter.value ?? false ? AppType.flutter : AppType.dart;
appIsFlutter.value == true ? AppType.flutter : AppType.dart;

final ValueNotifier<bool> appReady = ValueNotifier(false);

final ValueNotifier<List<AnalysisIssue>> analysisIssues = ValueNotifier([]);
final ValueNotifier<List<String>> packageImports = ValueNotifier([]);
final ValueNotifier<List<String>> importUrls = ValueNotifier([]);

final ValueNotifier<String> title = ValueNotifier('');

Expand Down Expand Up @@ -100,11 +99,16 @@ class AppModel {
currentDeltaDill.addListener(updateCanReload);

void updateShowReload() {
showReload.value = useNewDDC.value && (_appIsFlutter.value ?? false);
showReload.value = useNewDDC.value && (appIsFlutter.value ?? false);
}

void updateAppType() {
appIsFlutter.value = hasFlutterImports(importUrls.value);
}

useNewDDC.addListener(updateShowReload);
_appIsFlutter.addListener(updateShowReload);
appIsFlutter.addListener(updateShowReload);
importUrls.addListener(updateAppType);

_splitSubscription = splitDragStateManager.onSplitDragUpdated.listen((
SplitDragState value,
Expand Down Expand Up @@ -132,10 +136,10 @@ class AppModel {

void _recalcLayout() {
final hasConsoleText = !consoleNotifier.isEmpty;
final isFlutter = _appIsFlutter.value;
final usesPackageWeb = _usesPackageWeb;
final isFlutter = appIsFlutter.value;
final usesPackageWeb = hasPackageWebImport(importUrls.value);

if (isFlutter == null || usesPackageWeb == null) {
if (isFlutter == null) {
_layoutMode.value = LayoutMode.both;
} else if (usesPackageWeb) {
_layoutMode.value = LayoutMode.both;
Expand Down Expand Up @@ -524,9 +528,6 @@ class AppServices {
required bool isNewDDC,
required bool reload,
}) {
final appIsFlutter = hasFlutterWebMarker(javaScript, isNewDDC: isNewDDC);
appModel._appIsFlutter.value = appIsFlutter;
appModel._usesPackageWeb = hasPackageWebImport(dartSource);
appModel._recalcLayout();

_executionService?.execute(
Expand All @@ -535,7 +536,7 @@ class AppServices {
engineVersion: engineVersion,
reload: reload,
isNewDDC: isNewDDC,
isFlutter: appIsFlutter,
isFlutter: appModel.appIsFlutter.value ?? false,
);
}

Expand All @@ -551,7 +552,7 @@ class AppServices {
SourceRequest(source: appModel.sourceCodeController.text),
);
appModel.analysisIssues.value = results.issues;
appModel.packageImports.value = results.imports ?? [];
appModel.importUrls.value = results.imports;
} catch (error) {
appModel.analysisIssues.value = [
AnalysisIssue(
Expand All @@ -560,7 +561,7 @@ class AppServices {
location: Location(line: 0, column: 0),
),
];
appModel.packageImports.value = [];
appModel.importUrls.value = [];
}
}

Expand Down
25 changes: 4 additions & 21 deletions pkgs/dartpad_ui/lib/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,12 @@ RelativeRect calculatePopupMenuPosition(
);
}

bool hasFlutterWebMarker(String javaScript, {required bool isNewDDC}) {
const marker1 = 'window.flutterConfiguration';
if (javaScript.contains(marker1)) {
return true;
}
if (isNewDDC &&
javaScript.contains('defineLibrary(') &&
javaScript.contains('importLibrary("package:flutter/')) {
return true;
// define('dartpad_main', ['dart_sdk', 'flutter_web']
} else if (!isNewDDC &&
javaScript.contains("define('") &&
javaScript.contains("'flutter_web'")) {
return true;
}

return false;
bool hasFlutterImports(List<String> imports) {
return imports.any((import) => import.startsWith('package:flutter/'));
Copy link
Member Author

Choose a reason for hiding this comment

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

We can expand this list if we want (though I'm not sure what else would be on it).

}

bool hasPackageWebImport(String dartSource) {
// TODO(devoncarew): There are better ways to do this.
return dartSource.contains("import 'package:web/") ||
dartSource.contains('import "package:web/');
bool hasPackageWebImport(List<String> imports) {
return imports.any((import) => import.startsWith('package:web/'));
}

extension ColorExtension on Color {
Expand Down