fix(forms): remove the need for extra form scope

the forms/controls code refactored not to depend on events which forced
us to create new scope for each form element.

4 files changed, 192 insertions, 119 deletions

diff --git a/src/directive/form.js b/src/directive/form.js
index 96f7632b..27ba1044 100644
--- a/src/directive/form.js
+++ b/src/directive/form.js
@@ -22,49 +22,52 @@
  * of `FormController`.
  *
  */
-FormController.$inject = ['$scope', 'name'];
-function FormController($scope, name) {
+FormController.$inject = ['$scope', 'name', '$element'];
+function FormController($scope, name, element) {
   var form = this,
+      parentForm = element.parent().inheritedData('$formController'),
       errors = form.error = {};
 
   // publish the form into scope
   name(this);
 
-  $scope.$on('$destroy', function(event, widget) {
-    if (!widget) return;
+  if (parentForm) {
+    parentForm.$addControl(form);
+  }
 
-    if (widget.widgetId && form[widget.widgetId] === widget) {
-      delete form[widget.widgetId];
+  form.$addControl = function(control) {
+    if (control.name && !form.hasOwnProperty(control.name)) {
+      form[control.name] = control;
     }
-    forEach(errors, removeWidget, widget);
-  });
-
-  $scope.$on('$valid', function(event, error, widget) {
-    removeWidget(errors[error], error, widget);
+  }
 
-    if (equals(errors, {})) {
-      form.valid = true;
-      form.invalid = false;
+  form.$removeControl = function(control) {
+    if (control.name && form[control.name] === control) {
+      delete form[control.name];
     }
-  });
+    forEach(errors, cleanupControlErrors, control);
+  };
 
-  $scope.$on('$invalid', function(event, error, widget) {
-    addWidget(error, widget);
+  form.$setValidity = function(validationToken, isValid, control) {
+    if (isValid) {
+      cleanupControlErrors(errors[validationToken], validationToken, control);
 
-    form.valid = false;
-    form.invalid = true;
-  });
+      if (equals(errors, {})) {
+        form.valid = true;
+        form.invalid = false;
+      }
+    } else {
+      addControlError(validationToken, control);
 
-  $scope.$on('$viewTouch', function() {
+      form.valid = false;
+      form.invalid = true;
+    }
+  };
+
+  form.$setDirty = function() {
     form.dirty = true;
     form.pristine = false;
-  });
-
-  $scope.$on('$newFormControl', function(event, widget) {
-    if (widget.widgetId && !form.hasOwnProperty(widget.widgetId)) {
-      form[widget.widgetId] = widget;
-    }
-  });
+  }
 
   // init state
   form.dirty = false;
@@ -72,32 +75,40 @@ function FormController($scope, name) {
   form.valid = true;
   form.invalid = false;
 
-  function removeWidget(queue, errorKey, widget) {
+  function cleanupControlErrors(queue, validationToken, control) {
     if (queue) {
-      widget = widget || this; // so that we can be used in forEach;
+      control = control || this; // so that we can be used in forEach;
       for (var i = 0, length = queue.length; i < length; i++) {
-        if (queue[i] === widget) {
+        if (queue[i] === control) {
           queue.splice(i, 1);
           if (!queue.length) {
-            delete errors[errorKey];
+            delete errors[validationToken];
+
+            if (parentForm) {
+              parentForm.$setValidity(validationToken, true, form);
+            }
           }
         }
       }
     }
   }
 
-  function addWidget(errorKey, widget) {
-    var queue = errors[errorKey];
+  function addControlError(validationToken, control) {
+    var queue = errors[validationToken];
     if (queue) {
       for (var i = 0, length = queue.length; i < length; i++) {
-        if (queue[i] === widget) {
+        if (queue[i] === control) {
           return;
         }
       }
     } else {
-      errors[errorKey] = queue = [];
+      errors[validationToken] = queue = [];
+
+      if (parentForm) {
+        parentForm.$setValidity(validationToken, false, form);
+      }
     }
-    queue.push(widget);
+    queue.push(control);
   }
 }
 
@@ -107,12 +118,12 @@ function FormController($scope, name) {
  * @name angular.module.ng.$compileProvider.directive.form
  * @restrict EA
  *
- * @scope
  * @description
  * Directive that instantiates
  * {@link angular.module.ng.$compileProvider.directive.form.FormController FormController}.
  *
- * If `name` attribute is specified, the controller is published to the scope as well.
+ * If `name` attribute is specified, the form controller is published onto the current scope under
+ * this name.
  *
  * # Alias: `ng-form`
  *
@@ -164,28 +175,28 @@ function FormController($scope, name) {
       <doc:source>
        <script>
          function Ctrl($scope) {
-           $scope.text = 'guest';
+           $scope.userType = 'guest';
          }
        </script>
        <form name="myForm" ng-controller="Ctrl">
-         text: <input type="text" name="input" ng-model="text" required>
-         <span class="error" ng-show="myForm.input.error.REQUIRED">Required!</span>
-         <tt>text = {{text}}</tt><br/>
-         <tt>myForm.input.valid = {{myForm.input.valid}}</tt><br/>
-         <tt>myForm.input.error = {{myForm.input.error}}</tt><br/>
-         <tt>myForm.valid = {{myForm.valid}}</tt><br/>
-         <tt>myForm.error.REQUIRED = {{!!myForm.error.REQUIRED}}</tt><br/>
+         userType: <input name="input" ng-model="userType" required>
+         <span class="error" ng-show="myForm.input.error.REQUIRED">Required!</span><br>
+         <tt>userType = {{userType}}</tt><br>
+         <tt>myForm.input.valid = {{myForm.input.valid}}</tt><br>
+         <tt>myForm.input.error = {{myForm.input.error}}</tt><br>
+         <tt>myForm.valid = {{myForm.valid}}</tt><br>
+         <tt>myForm.error.REQUIRED = {{!!myForm.error.REQUIRED}}</tt><br>
         </form>
       </doc:source>
       <doc:scenario>
         it('should initialize to model', function() {
-         expect(binding('text')).toEqual('guest');
+         expect(binding('userType')).toEqual('guest');
          expect(binding('myForm.input.valid')).toEqual('true');
         });
 
         it('should be invalid if empty', function() {
-         input('text').enter('');
-         expect(binding('text')).toEqual('');
+         input('userType').enter('');
+         expect(binding('userType')).toEqual('');
          expect(binding('myForm.input.valid')).toEqual('false');
         });
       </doc:scenario>
@@ -195,7 +206,6 @@ var formDirective = [function() {
   return {
     name: 'form',
     restrict: 'E',
-    scope: true,
     inject: {
       name: 'accessor'
     },
@@ -203,7 +213,6 @@ var formDirective = [function() {
     compile: function() {
       return {
         pre: function(scope, formElement, attr, controller) {
-          formElement.data('$form', controller);
           formElement.bind('submit', function(event) {
             if (!attr.action) event.preventDefault();
           });
diff --git a/src/directive/input.js b/src/directive/input.js
index 7b95baaf..019bfaf0 100644
--- a/src/directive/input.js
+++ b/src/directive/input.js
@@ -131,7 +131,7 @@ var inputType = {
 
           it('should be invalid if over max', function() {
            input('value').enter('123');
-           expect(binding('value')).toEqual('12');
+           expect(binding('value')).toEqual('');
            expect(binding('myForm.input.valid')).toEqual('false');
           });
         </doc:scenario>
@@ -382,7 +382,7 @@ function textInputType(scope, element, attr, ctrl) {
   var pattern = attr.ngPattern,
       patternValidator;
 
-  var emit = function(regexp, value) {
+  var validate = function(regexp, value) {
     if (isEmpty(value) || regexp.test(value)) {
       ctrl.setValidity('PATTERN', true);
       return value;
@@ -396,7 +396,7 @@ function textInputType(scope, element, attr, ctrl) {
     if (pattern.match(/^\/(.*)\/$/)) {
       pattern = new RegExp(pattern.substr(1, pattern.length - 2));
       patternValidator = function(value) {
-        return emit(pattern, value)
+        return validate(pattern, value)
       };
     } else {
       patternValidator = function(value) {
@@ -405,7 +405,7 @@ function textInputType(scope, element, attr, ctrl) {
         if (!patternObj || !patternObj.test) {
           throw new Error('Expected ' + pattern + ' to be a RegExp but was ' + patternObj);
         }
-        return emit(patternObj, value);
+        return validate(patternObj, value);
       };
     }
 
@@ -676,7 +676,7 @@ function checkboxInputType(scope, element, attr, ctrl) {
 
         it('should be invalid if empty when required', function() {
           input('user.name').enter('');
-          expect(binding('user')).toEqual('{"last":"visitor","name":null}');
+          expect(binding('user')).toEqual('{"last":"visitor"}');
           expect(binding('myForm.userName.valid')).toEqual('false');
           expect(binding('myForm.valid')).toEqual('false');
         });
@@ -690,16 +690,16 @@ function checkboxInputType(scope, element, attr, ctrl) {
 
         it('should be invalid if less than required min length', function() {
           input('user.last').enter('xx');
-          expect(binding('user')).toEqual('{"last":"visitor","name":"guest"}');
+          expect(binding('user')).toEqual('{"name":"guest"}');
           expect(binding('myForm.lastName.valid')).toEqual('false');
           expect(binding('myForm.lastName.error')).toMatch(/MINLENGTH/);
           expect(binding('myForm.valid')).toEqual('false');
         });
 
-        it('should be valid if longer than max length', function() {
+        it('should be invalid if longer than max length', function() {
           input('user.last').enter('some ridiculously long name');
           expect(binding('user'))
-            .toEqual('{"last":"visitor","name":"guest"}');
+            .toEqual('{"name":"guest"}');
           expect(binding('myForm.lastName.valid')).toEqual('false');
           expect(binding('myForm.lastName.error')).toMatch(/MAXLENGTH/);
           expect(binding('myForm.valid')).toEqual('false');
@@ -748,13 +748,14 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
   this.modelValue = Number.NaN;
   this.parsers = [];
   this.formatters = [];
+  this.viewChangeListeners = [];
   this.error = {};
   this.pristine = true;
   this.dirty = false;
   this.valid = true;
   this.invalid = false;
   this.render = noop;
-  this.widgetId = $attr.name;
+  this.name = $attr.name;
 
 
   /**
@@ -763,32 +764,34 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
    * @methodOf angular.module.ng.$compileProvider.directive.ng-model.NgModelController
    *
    * @description
-   * Change the validity state, and notifies the form when the widget changes validity. (i.e. does
-   * not emit `$invalid` if given validator is already marked as invalid).
+   * Change the validity state, and notifies the form when the widget changes validity. (i.e. it
+   * does not notify form if given validator is already marked as invalid).
    *
-   * This method should be called by validators - ie the parser or formatter method.
+   * This method should be called by validators - i.e. the parser or formatter functions.
    *
-   * @param {string} name Name of the validator.
-   * @param {boolean} isValid Whether it should $emit `$valid` (true) or `$invalid` (false) event.
+   * @param {string} validationToken Name of the validator.
+   * @param {boolean} isValid Whether the current state is valid (true) or invalid (false).
    */
-  this.setValidity = function(name, isValid) {
+  this.setValidity = function(validationToken, isValid) {
 
-    if (!isValid && this.error[name]) return;
-    if (isValid && !this.error[name]) return;
+    if (!isValid && this.error[validationToken]) return;
+    if (isValid && !this.error[validationToken]) return;
 
     if (isValid) {
-      delete this.error[name];
+      delete this.error[validationToken];
       if (equals(this.error, {})) {
         this.valid = true;
         this.invalid = false;
       }
     } else {
-      this.error[name] = true;
+      this.error[validationToken] = true;
       this.invalid = true;
       this.valid = false;
     }
 
-    return $scope.$emit(isValid ? '$valid' : '$invalid', name, this);
+    if (this.$form) {
+      this.$form.$setValidity(validationToken, isValid, this);
+    }
   };
 
 
@@ -804,10 +807,10 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
    * For example {@link angular.module.ng.$compileProvider.directive.input input} or
    * {@link angular.module.ng.$compileProvider.directive.select select} directives call it.
    *
-   * It internally calls all `formatters` and if resulted value is valid, update the model and emits
-   * `$viewChange` event afterwards.
+   * It internally calls all `formatters` and if resulted value is valid, updates the model and
+   * calls all registered change listeners.
    *
-   * @param {string} value Value from the view
+   * @param {string} value Value from the view.
    */
   this.setViewValue = function(value) {
     this.viewValue = value;
@@ -816,17 +819,23 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', 'ngModel',
     if (this.pristine) {
       this.dirty = true;
       this.pristine = false;
-      $scope.$emit('$viewTouch', this);
+      if (this.$form) this.$form.$setDirty();
     }
 
     forEach(this.parsers, function(fn) {
       value = fn(value);
     });
 
-    if (isDefined(value) && this.model !== value) {
+    if (this.modelValue !== value) {
       this.modelValue = value;
       ngModel(value);
-      $scope.$emit('$viewChange', value, this);
+      forEach(this.viewChangeListeners, function(listener) {
+        try {
+          listener();
+        } catch(e) {
+          $exceptionHandler(e);
+        }
+      })
     }
   };
 
@@ -892,22 +901,28 @@ var ngModelDirective = [function() {
     inject: {
       ngModel: 'accessor'
     },
-    require: 'ngModel',
+    require: ['ngModel', '^?form'],
     controller: NgModelController,
-    link: function(scope, element, attr, ctrl) {
+    link: function(scope, element, attr, ctrls) {
       // notify others, especially parent forms
-      scope.$emit('$newFormControl', ctrl);
+
+      var modelCtrl = ctrls[0],
+          formCtrl = ctrls[1];
+
+      modelCtrl.$form = formCtrl;
+
+      if (formCtrl) formCtrl.$addControl(modelCtrl);
 
       forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
         scope.$watch(function() {
-          return ctrl[name];
+          return modelCtrl[name];
         }, function(value) {
           element[value ? 'addClass' : 'removeClass']('ng-' + name);
         });
       });
 
       element.bind('$destroy', function() {
-        scope.$emit('$destroy', ctrl);
+        if (formCtrl) formCtrl.$removeControl(modelCtrl);
       });
     }
   };
@@ -965,8 +980,8 @@ var ngModelDirective = [function() {
 var ngChangeDirective = valueFn({
   require: 'ngModel',
   link: function(scope, element, attr, ctrl) {
-    scope.$on('$viewChange', function(event, value, widget) {
-      if (ctrl === widget) scope.$eval(attr.ngChange);
+    ctrl.viewChangeListeners.push(function() {
+      scope.$eval(attr.ngChange);
     });
   }
 });
@@ -1044,7 +1059,7 @@ var requiredDirective = [function() {
       var validator = function(value) {
         if (attr.required && (isEmpty(value) || value === false)) {
           ctrl.setValidity('REQUIRED', false);
-          return null;
+          return;
         } else {
           ctrl.setValidity('REQUIRED', true);
           return value;
diff --git a/test/directive/formSpec.js b/test/directive/formSpec.js
index 45af04a4..c5e305d3 100644
--- a/test/directive/formSpec.js
+++ b/test/directive/formSpec.js
@@ -26,8 +26,8 @@ describe('form', function() {
 
   it('should instantiate form and attach it to DOM', function() {
     doc = $compile('<form>')(scope);
-    expect(doc.data('$form')).toBeTruthy();
-    expect(doc.data('$form') instanceof FormController).toBe(true);
+    expect(doc.data('$formController')).toBeTruthy();
+    expect(doc.data('$formController') instanceof FormController).toBe(true);
   });
 
 
@@ -78,15 +78,15 @@ describe('form', function() {
   });
 
 
-  it('should publish form to scope', function() {
+  it('should publish form to scope when name attr is defined', function() {
     doc = $compile('<form name="myForm"></form>')(scope);
     expect(scope.myForm).toBeTruthy();
-    expect(doc.data('$form')).toBeTruthy();
-    expect(doc.data('$form')).toEqual(scope.myForm);
+    expect(doc.data('$formController')).toBeTruthy();
+    expect(doc.data('$formController')).toEqual(scope.myForm);
   });
 
 
-  it('should allow name to be an expression', function() {
+  it('should allow form name to be an expression', function() {
     doc = $compile('<form name="obj.myForm"></form>')(scope);
 
     expect(scope.obj).toBeDefined();
@@ -108,7 +108,7 @@ describe('form', function() {
     var input = child.text;
 
     input.setValidity('MyError', false);
-    expect(parent.error.MyError).toEqual([input]);
+    expect(parent.error.MyError).toEqual([child]);
     expect(child.error.MyError).toEqual([input]);
 
     input.setValidity('MyError', true);
@@ -117,6 +117,41 @@ describe('form', function() {
   });
 
 
+  it('should support two forms on a single scope', function() {
+    doc = $compile(
+      '<div>' +
+        '<form name="formA">' +
+          '<input name="firstName" ng-model="firstName" required>' +
+        '</form>' +
+        '<form name="formB">' +
+          '<input name="lastName" ng-model="lastName" required>' +
+        '</form>' +
+      '</div>'
+    )(scope);
+
+    scope.$apply();
+
+    expect(scope.formA.error.REQUIRED.length).toBe(1);
+    expect(scope.formA.error.REQUIRED).toEqual([scope.formA.firstName]);
+    expect(scope.formB.error.REQUIRED.length).toBe(1);
+    expect(scope.formB.error.REQUIRED).toEqual([scope.formB.lastName]);
+
+    var inputA = doc.find('input').eq(0),
+        inputB = doc.find('input').eq(1);
+
+    inputA.val('val1');
+    browserTrigger(inputA, 'blur');
+    inputB.val('val2');
+    browserTrigger(inputB, 'blur');
+
+    expect(scope.firstName).toBe('val1');
+    expect(scope.lastName).toBe('val2');
+
+    expect(scope.formA.error.REQUIRED).toBeUndefined();
+    expect(scope.formB.error.REQUIRED).toBeUndefined();
+  });
+
+
   it('should chain nested forms in repeater', function() {
     doc = jqLite(
        '<ng:form name=parent>' +
@@ -141,7 +176,7 @@ describe('form', function() {
     input.setValidity('myRule', false);
     expect(input.error.myRule).toEqual(true);
     expect(child.error.myRule).toEqual([input]);
-    expect(parent.error.myRule).toEqual([input]);
+    expect(parent.error.myRule).toEqual([child]);
 
     input.setValidity('myRule', true);
     expect(parent.error.myRule).toBeUndefined();
diff --git a/test/directive/inputSpec.js b/test/directive/inputSpec.js
index 653dfdc6..724b9fbe 100644
--- a/test/directive/inputSpec.js
+++ b/test/directive/inputSpec.js
@@ -30,18 +30,18 @@ describe('NgModelController', function() {
     expect(ctrl.formatters).toEqual([]);
     expect(ctrl.parsers).toEqual([]);
 
-    expect(ctrl.widgetId).toBe('testAlias');
+    expect(ctrl.name).toBe('testAlias');
   });
 
 
   describe('setValidity', function() {
 
-    it('should emit $invalid only when $valid', function() {
-      var spy = jasmine.createSpy('$invalid');
-      scope.$on('$invalid', spy);
+    it('should propagate invalid to the parent form only when valid', function() {
+      var spy = jasmine.createSpy('setValidity');
+      ctrl.$form = {$setValidity: spy};
 
       ctrl.setValidity('ERROR', false);
-      expect(spy).toHaveBeenCalledOnce();
+      expect(spy).toHaveBeenCalledOnceWith('ERROR', false, ctrl);
 
       spy.reset();
       ctrl.setValidity('ERROR', false);
@@ -78,15 +78,17 @@ describe('NgModelController', function() {
 
 
     it('should emit $valid only when $invalid', function() {
-      var spy = jasmine.createSpy('$valid');
-      scope.$on('$valid', spy);
+      var spy = jasmine.createSpy('setValidity');
+      ctrl.$form = {$setValidity: spy};
 
       ctrl.setValidity('ERROR', true);
       expect(spy).not.toHaveBeenCalled();
 
       ctrl.setValidity('ERROR', false);
+      expect(spy).toHaveBeenCalledOnceWith('ERROR', false, ctrl);
+      spy.reset();
       ctrl.setValidity('ERROR', true);
-      expect(spy).toHaveBeenCalledOnce();
+      expect(spy).toHaveBeenCalledOnceWith('ERROR', true, ctrl);
     });
   });
 
@@ -118,10 +120,10 @@ describe('NgModelController', function() {
     });
 
 
-    it('should fire $viewChange only if value changed and is valid', function() {
-      var spy = jasmine.createSpy('$viewChange');
-      scope.$on('$viewChange', spy);
-
+    it('should fire viewChangeListeners when the value changes in the view (even if invalid)',
+        function() {
+      var spy = jasmine.createSpy('viewChangeListener');
+      ctrl.viewChangeListeners.push(spy);
       ctrl.setViewValue('val');
       expect(spy).toHaveBeenCalledOnce();
       spy.reset();
@@ -129,13 +131,25 @@ describe('NgModelController', function() {
       // invalid
       ctrl.parsers.push(function() {return undefined;});
       ctrl.setViewValue('val');
-      expect(spy).not.toHaveBeenCalled();
+      expect(spy).toHaveBeenCalledOnce();
     });
 
 
-    it('should only fire $viewTouch when pristine', function() {
-      var spy = jasmine.createSpy('$viewTouch');
-      scope.$on('$viewTouch', spy);
+    it('should reset the model when the view is invalid', function() {
+      ctrl.setViewValue('aaaa');
+      expect(ctrl.modelValue).toBe('aaaa');
+
+      // add a validator that will make any input invalid
+      ctrl.parsers.push(function() {return undefined;});
+      expect(ctrl.modelValue).toBe('aaaa');
+      ctrl.setViewValue('bbbb');
+      expect(ctrl.modelValue).toBeUndefined;
+    });
+
+
+    it('should call parentForm.setDirty only when pristine', function() {
+      var spy = jasmine.createSpy('setDirty');
+      ctrl.$form = {$setDirty: spy};
 
       ctrl.setViewValue('');
       expect(ctrl.pristine).toBe(false);
@@ -278,14 +292,14 @@ describe('input', function() {
   });
 
 
-  it('should call $destroy on element remove', function() {
-    compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');
+  it('should cleanup it self from the parent form', function() {
+    compileInput('<input ng-model="name" name="alias" required>');
 
-    var spy = jasmine.createSpy('on destroy');
-    scope.$on('$destroy', spy);
+    scope.$apply();
+    expect(scope.form.error.REQUIRED.length).toBe(1);
 
     inputElm.remove();
-    expect(spy).toHaveBeenCalled();
+    expect(scope.form.error.REQUIRED).toBeUndefined();
   });
 
 
@@ -450,7 +464,7 @@ describe('input', function() {
 
   describe('number', function() {
 
-    it('should not update model if view invalid', function() {
+    it('should reset the model if view is invalid', function() {
       compileInput('<input type="number" ng-model="age"/>');
 
       scope.$apply(function() {
@@ -467,7 +481,7 @@ describe('input', function() {
 
       changeInputValueTo('123X');
       expect(inputElm.val()).toBe('123X');
-      expect(scope.age).toBe(123);
+      expect(scope.age).toBeUndefined();
       expect(inputElm).toBeInvalid();
     });
 
@@ -588,7 +602,7 @@ describe('input', function() {
       expect(widget.error.EMAIL).toBeUndefined();
 
       changeInputValueTo('invalid@');
-      expect(scope.email).toBe('vojta@google.com');
+      expect(scope.email).toBeUndefined();
       expect(inputElm).toBeInvalid();
       expect(widget.error.EMAIL).toBeTruthy();
     });
@@ -616,7 +630,7 @@ describe('input', function() {
       expect(widget.error.URL).toBeUndefined();
 
       changeInputValueTo('invalid.com');
-      expect(scope.url).toBe('http://www.something.com');
+      expect(scope.url).toBeUndefined();
       expect(inputElm).toBeInvalid();
       expect(widget.error.URL).toBeTruthy();
     });