[WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps #771

chalin · 2014-03-20T02:49:30Z

chalin · 2014-03-20T03:12:57Z

One last(?) attempt at providing a general solution to partly address some of the concerns raised in #705, and to salvage some lecture material and the AngularDart tutorial ... without requiring each user to write a bunch of "utility" filters, like toString. The goal, now that #763 has been closed, is to propose something that (I believe) is consistent with Angular design goals.

Of course these filters might not be as efficient as custom filters, but users are always free to write their own. These may even subsume lowercase and uppercase.

mhevery · 2014-03-20T05:23:35Z

Yes this is great.

Look at https://github.com/mhevery/angular.dart/tree/no-mirrors notice how I have moved all of the mirrors to a separate import. We need to do this so that we have reasonable size outputs. Transforms which generate all reflective code are coming here https://github.com/mhevery/angular.dart/tree/transformers Your PR needs to work with these changes which will be merged in the next few days.

vicb · 2014-03-20T09:10:21Z

lib/filter/pure.dart

+
+/**
+ * General purpose filter used to perform a "get field" operation on the named
+ * field of the filter argument. Use this when the "get field" operation is


should be "filtered argument"

vicb · 2014-03-20T09:18:06Z

Should field & method be merged into a single filter ?
Could you please update the commit comment with "Closes #..., Relates to #...,". Thanks.

vicb · 2014-03-20T09:18:31Z

lib/filter/pure.dart

+class ApplyPureMethodFilter implements Function {
+  dynamic call(dynamic receiver, String functionName, [List args]) {
+    if (receiver == null) return null;
+    var mirror = reflect(receiver),


chalin · 2014-03-20T12:40:23Z

Should field & method be merged into a single filter ?

Not sure what to call it then.

vicb · 2014-03-20T12:47:22Z

Python calls that attr

vicb · 2014-03-20T12:49:59Z

I guess Dart would call that "method" (cf noSuchMethod). It makes sense if you see a property as a (getter, setter) pair but it would not be very intuitive.

I wouldn't mind:

access
field
attr(ibute)

In order of preference

chalin · 2014-03-20T12:54:29Z

How about member? That is what fields and methods are collectively referred to in Dart---cf. the "." operator name in DUnR.

vicb · 2014-03-20T12:56:58Z

member could be good as well...

But in the end the solution is maybe to keep separate directives, the common code is short and the arguments are different anyway.

chalin · 2014-03-20T13:26:43Z

Btw, how about renaming id to something like observe?

chalin · 2014-03-20T15:40:27Z

Updated:

Now properly handles namedArgs.
Added tests.
Renamed id filter to observe
Updated API docs.

Once / if support is added for the Dart/Jinja-like filter argument syntax, then the method filter should be improved to handle arguments a la Dart.

I will work on trying to create a version of pure.dart over Misko's no-mirrors branch (or should I just wait for it to get merged into master?).

vicb · 2014-03-20T15:55:32Z

lib/filter/pure.dart

+      args = const [];
+    }
+    Map<Symbol,dynamic> _namedArgs;
+    if (namedArgs != null) {


I don't have time to check right now but couldn't this be simpler ?

We should be using final where possible, see https://groups.google.com/a/dartlang.org/forum/#!topic/misc/qccnux8c6BE

Do you really need to copy the map ? the may be Map.from

Couldn't invoke be invoked with null / {} as a last arg ?

chalin · 2014-03-20T16:15:48Z

Simplified. (You were correct, it accepts an empty map.)

vicb · 2014-03-20T19:51:44Z

lib/filter/pure.dart

+    final Map<Symbol,dynamic> _namedArgs = namedArgs == null ?
+        const <Symbol,dynamic>{} : <Symbol,dynamic>{};
+    if (namedArgs != null) {
+      namedArgs.forEach((k,v) => _namedArgs.putIfAbsent(new Symbol(k), () => v));


_namedArgs[new S] = v ?

… to observe lists and maps Closes dart-archive#359, dart-archive#394, dart-archive#397, dart-archive#757. Relates to dart-archive#772, dart-archive#773.

mhevery · 2014-04-01T04:19:56Z

lib/filter/module.dart

@@ -1,6 +1,7 @@
 library angular.filter;

 import 'dart:convert' show JSON;
+import 'dart:mirrors';


Mirrors can not be used in Angular. See
https://github.com/angular/angular.dart/blob/master/lib/angular_dynamic.dart
https://github.com/angular/angular.dart/blob/master/lib/angular_static.dart
and
https://github.com/angular/angular.dart/blob/master/example/pubspec.yaml#L10

Basically mirrors get replaced with generated code using the transformers during the compilation phase.

mhevery · 2014-04-01T04:21:32Z

I am skeptical that we will be able to get this in due to the way this uses mirrors. We now have a compile step which removes all the mirrors from the code base using the transformers, and this code would break the transformers. To get this in, we need to solve this in a way which does not use mirrors API.

chalin · 2014-04-02T20:13:59Z

I was waiting for your the new angular_dynamic/static code to be in before looking into this further. Now that it is part of master I will have a look. @vicb's stringify filter will achieve the effect of the observe filter of this PR. Hence, observe can be dropped. You are likely right that the other two filters might not be usable given that they are generic. Anyhow, I'll have a look as soon as I have finished the date work.

mhevery · 2014-04-18T03:54:04Z

I am going to close this due to inactivity. If you can get this working without mirrors then resubmit as new PR.

chalin mentioned this pull request Mar 20, 2014

feat(change_detection): treat List properties as pure #763

Closed

chalin added cla: yes and removed cla: no labels Mar 20, 2014

vicb reviewed Mar 20, 2014
View reviewed changes

chalin changed the title ~~feat(filters): add general purpose filters in support of pure fields and methods~~ feat(filters): add filters in support of pure fields and methods, and to observe lists and maps Mar 20, 2014

vicb reviewed Mar 20, 2014
View reviewed changes

chalin mentioned this pull request Mar 20, 2014

Observable support in change detection #773

Closed

vicb reviewed Mar 20, 2014
View reviewed changes

vicb and others added 2 commits March 20, 2014 21:22

chore(release): v0.10.0 pachyderm-humidification

f4527ea

feat(filters): add filters in support of pure fields and methods, and…

655dc14

… to observe lists and maps Closes dart-archive#359, dart-archive#394, dart-archive#397, dart-archive#757. Relates to dart-archive#772, dart-archive#773.

mhevery added the DON'T MERGE label Mar 25, 2014

mhevery reviewed Apr 1, 2014
View reviewed changes

chalin changed the title ~~feat(filters): add filters in support of pure fields and methods, and to observe lists and maps~~ [WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps Apr 2, 2014

mhevery closed this Apr 18, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps #771

[WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps #771

chalin commented Mar 20, 2014

chalin commented Mar 20, 2014

mhevery commented Mar 20, 2014

vicb Mar 20, 2014

vicb commented Mar 20, 2014

vicb Mar 20, 2014

chalin commented Mar 20, 2014

vicb commented Mar 20, 2014

vicb commented Mar 20, 2014

chalin commented Mar 20, 2014

vicb commented Mar 20, 2014

chalin commented Mar 20, 2014

chalin commented Mar 20, 2014

vicb Mar 20, 2014

chalin commented Mar 20, 2014

vicb Mar 20, 2014

mhevery Apr 1, 2014

mhevery commented Apr 1, 2014

chalin commented Apr 2, 2014

mhevery commented Apr 18, 2014

[WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps #771

[WIP] feat(filters): add filters in support of pure fields and methods, and to observe lists and maps #771

Conversation

chalin commented Mar 20, 2014

chalin commented Mar 20, 2014

mhevery commented Mar 20, 2014

vicb Mar 20, 2014

Choose a reason for hiding this comment

vicb commented Mar 20, 2014

vicb Mar 20, 2014

Choose a reason for hiding this comment

chalin commented Mar 20, 2014

vicb commented Mar 20, 2014

vicb commented Mar 20, 2014

chalin commented Mar 20, 2014

vicb commented Mar 20, 2014

chalin commented Mar 20, 2014

chalin commented Mar 20, 2014

vicb Mar 20, 2014

Choose a reason for hiding this comment

chalin commented Mar 20, 2014

vicb Mar 20, 2014

Choose a reason for hiding this comment

mhevery Apr 1, 2014

Choose a reason for hiding this comment

mhevery commented Apr 1, 2014

chalin commented Apr 2, 2014

mhevery commented Apr 18, 2014