Merge pull request #118 from sandersky/master

Add setting to enforce propTypes on components

addon/extensions/component-prop-types.js

@@ -1,7 +1,8 @@
 import Ember from 'ember'
-const {Component} = Ember
+const {Component, isArray} = Ember
 
-import PropTypeMixin from '../mixins/prop-types'
+import PropTypeMixin, {settings} from '../mixins/prop-types'
+import {logger} from '../utils/prop-types'
 
 /**
  * @module
@@ -10,4 +11,18 @@ import PropTypeMixin from '../mixins/prop-types'
 /**
  * @memberof ember/Component#
  */
-Component.reopen(PropTypeMixin)
+Component.reopen(PropTypeMixin, {
+  init () {
+    if (settings.requireComponentPropTypes) {
+      const propTypes = this.get('propTypes')
+
+      if (!isArray(propTypes) || propTypes.length === 0) {
+        logger.warn(
+          this, 'propTypes is required for components', settings.throwErrors
+        )
+      }
+    }
+
+    this._super(...arguments)
+  }
+})

addon/mixins/prop-types.js

@@ -8,6 +8,9 @@ import config from 'ember-get-config'
 import PropTypes, {getDef, logger, validators} from '../utils/prop-types'
 
 export const settings = {
+  requireComponentPropTypes: getWithDefault(
+    config, 'ember-prop-types.requireComponentPropTypes', false
+  ),
   spreadProperty: get(config, 'ember-prop-types.spreadProperty'),
   throwErrors: getWithDefault(config, 'ember-prop-types.throwErrors', false),
   validate: get(config, 'ember-prop-types.validate'),
@@ -99,22 +102,34 @@ export default Mixin.create({
   init () {
     helpers.validatePropTypes(this)
 
-    const keys = Object.keys(this)
+    // Note defaults is a concatenated property so this is actually an array
+    // of getDefaultProps() methods all the way up the inheritance chain
     const defaults = this.get('getDefaultProps')
-    defaults.forEach((propsFunction) => {
-      if (typeOf(propsFunction) !== 'function') {
-        return
-      }
 
-      const defaultProps = propsFunction.apply(this)
-      keys.forEach((key) => {
-        if (this.get(key) !== undefined) {
-          delete defaultProps[key]
-        }
+    const defaultProps = {} // Concatenated list of all defaults
+
+    defaults
+      .forEach((propsFunction) => {
+        // If for some reason getDefaultProps() isn't a function we'll simply
+        // ignore it. In the future we might want to actually assert an error
+        // here.
+        if (typeOf(propsFunction) !== 'function') return
+
+        // Now let's actually call the getDefaultProps() method and update our
+        // concatenated defaults with the results of this method call.
+        Object.assign(defaultProps, propsFunction.apply(this))
       })
 
-      this.setProperties(defaultProps)
-    })
+    // Now iterate defaults and remove any properties that already have
+    // values set on this instance.
+    Object.keys(defaultProps)
+      .forEach((key) => {
+        if (this.get(key) !== undefined) delete defaultProps[key]
+      })
+
+    // Time to go ahead and apply the defaults
+    this.setProperties(defaultProps)
+
     this._super(...arguments)
   }
 })

bower.json

@@ -1,15 +1,10 @@
 {
   "name": "ember-prop-types",
   "dependencies": {
-    "ember": "^2.11.0",
-    "ember-cli-shims": "0.1.3",
-    "ember-mocha-adapter": "~0.3.1",
-    "ember-mocha": "0.8.11",
     "prism": "^1.5.1",
     "sinon-chai": "^2.8.0"
   },
   "devDependencies": {
-    "blanket": "5e94fc30f2e694bb5c3718ddcbf60d467f4b4d26",
     "sinonjs": "1.17.1"
   }
 }

package.json

@@ -31,16 +31,17 @@
   "license": "MIT",
   "devDependencies": {
     "broccoli-asset-rev": "^2.5.0",
-    "ember-cli": "^2.11.0",
+    "ember-cli": "^2.12.0",
     "ember-cli-app-version": "^2.0.1",
     "ember-cli-chai": "0.3.2",
-    "ember-cli-code-coverage": "0.3.11",
+    "ember-cli-code-coverage": "0.3.12",
     "ember-cli-dependency-checker": "^1.3.0",
     "ember-cli-google-fonts": "^2.3.1",
     "ember-cli-htmlbars": "^1.1.1",
-    "ember-cli-htmlbars-inline-precompile": "0.3.6",
+    "ember-cli-htmlbars-inline-precompile": "0.4.0",
     "ember-cli-inject-live-reload": "^1.4.1",
-    "ember-cli-mocha": "0.13.2",
+    "ember-cli-mocha": "0.13.3",
+    "ember-cli-shims": "^1.1.0",
     "ember-cli-test-loader": "^1.1.1",
     "ember-cli-uglify": "^1.2.0",
     "ember-disable-prototype-extensions": "^1.1.0",
@@ -49,9 +50,10 @@
     "ember-load-initializers": "0.6.3",
     "ember-prism": "0.0.8",
     "ember-resolver": "^2.1.1",
-    "ember-sinon": "0.6.0",
+    "ember-sinon": "0.7.0",
+    "ember-source": "^2.12.1",
     "ember-spread": "^1.0.0",
-    "ember-test-utils": "^1.10.3",
+    "ember-test-utils": "^4.0.0",
     "loader.js": "^4.1.0"
   },
   "keywords": [
@@ -65,4 +67,4 @@
   "ember-addon": {
     "configPath": "tests/dummy/config"
   }
-}
+}

tests/dummy/app/routes/application.js

@@ -27,6 +27,9 @@ export default Component.extend(PropTypeMixin, {
 
 const config = `
 'ember-prop-types': {
+  // When true components will throw an error if they are missing propTypes. (Default is false)
+  requireComponentPropTypes: true,
+
   // Validate properties coming from a spread property (default is undefined)
   spreadProperty: 'options',
 

tests/index.html

@@ -13,7 +13,6 @@
     <link rel="stylesheet" href="{{rootURL}}assets/vendor.css">
     <link rel="stylesheet" href="{{rootURL}}assets/dummy.css">
     <link rel="stylesheet" href="{{rootURL}}assets/test-support.css">
-    <style>#blanket-main { position: relative; z-index: 99999; }</style>
 
     {{content-for 'head-footer'}}
     {{content-for 'test-head-footer'}}

tests/unit/initializers/component-prop-types-test.js

@@ -4,11 +4,11 @@
 import {expect} from 'chai'
 import Ember from 'ember'
 const {Application, Component, run} = Ember
-import {beforeEach, describe, it} from 'mocha'
+import {after, before, beforeEach, describe, it} from 'mocha'
 
 import {createComponent} from 'dummy/tests/helpers/ember-prop-types'
 import {initialize} from 'ember-prop-types/initializers/component-prop-types'
-import PropTypesMixin from 'ember-prop-types/mixins/prop-types'
+import PropTypesMixin, {settings} from 'ember-prop-types/mixins/prop-types'
 
 describe('Unit / Initializers / component-prop-types', function () {
   let container, application
@@ -33,4 +33,138 @@ describe('Unit / Initializers / component-prop-types', function () {
       expect(PropTypesMixin.detect(component)).to.equal(true)
     })
   })
+
+  describe('when requireComponentPropTypes is set to true', function () {
+    let originalRequireComponentPropTypes
+
+    before(function () {
+      originalRequireComponentPropTypes = settings.requireComponentPropTypes
+      settings.requireComponentPropTypes = true
+    })
+
+    after(function () {
+      settings.requireComponentPropTypes = originalRequireComponentPropTypes
+    })
+
+    describe('when throwErrors is set to true', function () {
+      let originalThrowErrors
+
+      before(function () {
+        originalThrowErrors = settings.throwErrors
+        settings.throwErrors = true
+      })
+
+      after(function () {
+        settings.throwErrors = originalThrowErrors
+      })
+
+      it('does not throw error when component has propTypes', function () {
+        expect(() => {
+          createComponent(Component.extend({
+            propTypes: {}
+          }))
+        }).not.to.throw()
+      })
+
+      it('throws error when component does not have propTypes', function () {
+        expect(() => {
+          createComponent(Component)
+        }).to.throw()
+      })
+    })
+
+    describe('when throwErrors is set to false', function () {
+      let originalThrowErrors
+
+      before(function () {
+        originalThrowErrors = settings.throwErrors
+        settings.throwErrors = false
+      })
+
+      after(function () {
+        settings.throwErrors = originalThrowErrors
+      })
+
+      it('does not throw error when component has propTypes', function () {
+        expect(() => {
+          createComponent(Component.extend({
+            propTypes: {}
+          }))
+        }).not.to.throw()
+      })
+
+      it('does not throw error when component does not have propTypes', function () {
+        expect(() => {
+          createComponent(Component)
+        }).not.to.throw()
+      })
+    })
+  })
+
+  describe('when requireComponentPropTypes is set to false', function () {
+    let originalRequireComponentPropTypes
+
+    before(function () {
+      originalRequireComponentPropTypes = settings.requireComponentPropTypes
+      settings.requireComponentPropTypes = false
+    })
+
+    after(function () {
+      settings.requireComponentPropTypes = originalRequireComponentPropTypes
+    })
+
+    describe('when throwErrors is set to true', function () {
+      let originalThrowErrors
+
+      before(function () {
+        originalThrowErrors = settings.throwErrors
+        settings.throwErrors = true
+      })
+
+      after(function () {
+        settings.throwErrors = originalThrowErrors
+      })
+
+      it('does not throw error when component has propTypes', function () {
+        expect(() => {
+          createComponent(Component.extend({
+            propTypes: {}
+          }))
+        }).not.to.throw()
+      })
+
+      it('does not throw error when component does not have propTypes', function () {
+        expect(() => {
+          createComponent(Component)
+        }).not.to.throw()
+      })
+    })
+
+    describe('when throwErrors is set to false', function () {
+      let originalThrowErrors
+
+      before(function () {
+        originalThrowErrors = settings.throwErrors
+        settings.throwErrors = false
+      })
+
+      after(function () {
+        settings.throwErrors = originalThrowErrors
+      })
+
+      it('does not throw error when component has propTypes', function () {
+        expect(() => {
+          createComponent(Component.extend({
+            propTypes: {}
+          }))
+        }).not.to.throw()
+      })
+
+      it('does not throw error when component does not have propTypes', function () {
+        expect(() => {
+          createComponent(Component)
+        }).not.to.throw()
+      })
+    })
+  })
 })