fix(views): Better validation of view declarations (throw when there are state-level and view-level conflicts)

Closes #3340

Author: christopherthielen

src/statebuilders/views.ts

@@ -37,10 +37,20 @@ export function ng1ViewsBuilder(state: StateObject) {
       ctrlKeys = ['controller', 'controllerProvider', 'controllerAs', 'resolveAs'],
       compKeys = ['component', 'bindings', 'componentProvider'],
       nonCompKeys = tplKeys.concat(ctrlKeys),
-      allKeys = compKeys.concat(nonCompKeys);
+      allViewKeys = compKeys.concat(nonCompKeys);
+
+  // Do not allow a state to have both state-level props and also a `views: {}` property.
+  // A state without a `views: {}` property can declare properties for the `$default` view as properties of the state.
+  // However, the `$default` approach should not be mixed with a separate `views: ` block.
+  if (isDefined(state.views) && hasAnyKey(allViewKeys, state)) {
+    throw new Error(`State '${state.name}' has a 'views' object. ` +
+        `It cannot also have "view properties" at the state level.  ` +
+        `Move the following properties into a view (in the 'views' object): ` +
+        ` ${allViewKeys.filter(key => isDefined(state[key])).join(", ")}`);
+  }
 
   let views: { [key: string]: Ng1ViewDeclaration } = {},
-      viewsObject = state.views || { "$default": pick(state, allKeys) };
+      viewsObject = state.views || { "$default": pick(state, allViewKeys) };
 
   forEach(viewsObject, function (config: Ng1ViewDeclaration, name: string) {
     // Account for views: { "": { template... } }
@@ -51,6 +61,7 @@ export function ng1ViewsBuilder(state: StateObject) {
     // Make a shallow copy of the config object
     config = extend({}, config);
 
+    // Do not allow a view to mix props for component-style view with props for template/controller-style view
     if (hasAnyKey(compKeys, config) && hasAnyKey(nonCompKeys, config)) {
       throw new Error(`Cannot combine: ${compKeys.join("|")} with: ${nonCompKeys.join("|")} in stateview: '${name}@${state.name}'`);
     }

test/ng1StateBuilderSpec.ts

@@ -23,7 +23,7 @@ describe('Ng1 StateBuilder', function() {
     expect(built.$default).toEqualData({ templateUrl: "/foo.html", controller: "FooController", resolveAs: '$resolve' });
   });
 
-  it('It should use the views object to build views, when defined, function() {
+  it('It should use the views object to build views, when defined', function() {
     var config = { a: { foo: "bar", controller: "FooController" } };
     let builtViews = builder.builder('views')({ parent: parent, views: config });
     expect(builtViews.a.foo).toEqualData(config.a.foo);

test/viewDirectiveSpec.ts

@@ -403,22 +403,16 @@ describe('uiView', function () {
       expect(_scope.$ctrl.$resolve.user).toBe('joeschmoe');
     }));
 
-    it('should use the view-level resolveAs over the state-level resolveAs', inject(function ($state, $q, $timeout) {
+    it('should not allow both view-level resolveAs and state-level resolveAs on the same state', inject(function ($state, $q, $timeout) {
       let views = {
         "$default": {
           controller: controller,
           template: '{{$$$resolve.user}}',
           resolveAs: '$$$resolve'
         }
       };
-      let state = angular.extend({}, _state, { resolveAs: 'foo', views: views })
-      $stateProvider.state(state);
-      elem.append($compile('<div><ui-view></ui-view></div>')(scope));
-
-      $state.transitionTo('resolve'); $q.flush(); $timeout.flush();
-      expect(elem.text()).toBe('joeschmoe');
-      expect(_scope.$$$resolve).toBeDefined();
-      expect(_scope.$$$resolve.user).toBe('joeschmoe');
+      let state = angular.extend({}, _state, { resolveAs: 'foo', views: views });
+      expect(() => $stateProvider.state(state)).toThrowError(/resolveAs/);
     }));
   });