Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Unified Diff: lib/src/observable_transform.dart

Issue 20863002: Introduce boot.js: this finally makes it possible to load and run Todomvc (Closed) Base URL: git@github.com:dart-lang/web-ui.git@master
Patch Set: Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: lib/src/observable_transform.dart
diff --git a/lib/src/observable_transform.dart b/lib/src/observable_transform.dart
index 938b4da9a573a858a8f9e4905de3fb60c13e130d..a8bb350747625be9e60cfdd0b40c3e806d87fd16 100644
--- a/lib/src/observable_transform.dart
+++ b/lib/src/observable_transform.dart
@@ -86,6 +86,49 @@ void transformClass(ClassDeclaration cls, TextEditTransaction code,
_getSpan(file, cls));
}
+ // We'd like to track whether observable was declared explicitly, otherwise
+ // report a warning later below. Because we don't have type analysis (only
+ // syntactic understanding of the code), we only report warnings that are
+ // known to be true.
+ var declaresObservable = false;
+ if (cls.extendsClause != null) {
+ var id = _getSimpleIdentifier(cls.extendsClause.superclass.name);
+ if (id.name == 'ObservableBase') {
+ code.edit(id.offset, id.end, 'ChangeNotifierBase');
+ declaresObservable = true;
+ } else if (id.name == 'ChangeNotifierBase') {
+ declaresObservable = true;
+ } else if (id.name != 'PolymerElement' && id.name != 'Object') {
Jennifer Messerly 2013/07/31 01:51:33 include CustomElement too?
Siggi Cherem (dart-lang) 2013/07/31 02:17:29 Done
+ // TODO(sigmund): this is conservative, consider using type-resolution to
+ // improve this check.
+ declaresObservable = true;
+ }
+ }
+
+ if (cls.withClause != null) {
+ for (var type in cls.withClause.mixinTypes) {
+ var id = _getSimpleIdentifier(type.name);
+ if (id.name == 'ObservableMixin') {
+ code.edit(id.offset, id.end, 'ChangeNotifierMixin');
+ declaresObservable = true;
+ break;
+ } else if (id.name == 'ChangeNotifierMixin') {
+ declaresObservable = true;
+ break;
+ } else {
+ // TODO(sigmund): this is conservative, consider using type-resolution to
Jennifer Messerly 2013/07/31 01:51:33 long line
Siggi Cherem (dart-lang) 2013/07/31 02:17:29 Done.
+ // improve this check.
+ declaresObservable = true;
+ }
+ }
+ }
+
+ if (!declaresObservable && cls.implementsClause != null) {
+ // TODO(sigmund): consider adding type-resolution to give a more precise
+ // answer.
+ declaresObservable = true;
+ }
+
// Track fields that were transformed.
var instanceFields = new Set<String>();
var getters = new List<String>();
@@ -103,6 +146,14 @@ void transformClass(ClassDeclaration cls, TextEditTransaction code,
continue;
}
if (hasObservable(member)) {
+ if (!declaresObservable) {
+ messages.warning('Observable fields should be put in an observable'
+ ' objects. Please declare that this class extends from '
+ 'ObservableBase, includes ObservableMixin, or implements '
+ 'Observable.',
+ _getSpan(file, member));
+
+ }
transformFields(member.fields, code, member.offset, member.end);
var names = member.fields.variables.map((v) => v.name.name);
@@ -140,6 +191,9 @@ void transformClass(ClassDeclaration cls, TextEditTransaction code,
}
}
+SimpleIdentifier _getSimpleIdentifier(Identifier id) =>
+ id is PrefixedIdentifier ? id.identifier : id;
+
/**
* Generates `getValueWorkaround` and `setValueWorkaround`. These will go away

Powered by Google App Engine
This is Rietveld 408576698