Repository: ember-cli/eslint-plugin-ember
Pull request #256 (2018-05-23 08:42:47)

Add rule to disallow mixins
Mixins can cause a lot of headaches, and it would be nice to have a rule that disallowed them.

I left most of the heavy lifting of explaining why mixins can be bad to two reat blog posts; figured I didn't need to reinvent the wheel. Feedback especially appreciate on the doc for this rule: are the code examples easy to understand, etc.

cc: @scalvert

scalvert: @rwjblue agree that we definitely want to discourage mixin _usage_. That said, in an effort to stop adding new mixins, this rule seems like a good first step.
rwjblue: > That said, in an effort to stop adding new mixins, this rule seems like a good first step.

Yep, I hope I wasn't confusing there. I think this rule is good generally speaking, just not "the end" ...

Patch:
diff --git a/.gitignore b/.gitignore
index 1a4da1b3..9e925002 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,4 @@ node_modules
 coverage
 npm-debug.log
 *.swp
+.vscode
\ No newline at end of file
diff --git a/docs/rules/no-new-mixins.md b/docs/rules/no-new-mixins.md
new file mode 100644
index 00000000..acc1e940
--- /dev/null
+++ b/docs/rules/no-new-mixins.md
@@ -0,0 +1,54 @@
+## Don't create new Mixins
+
+### Rule name: `no-new-mixins`
+
+Using mixins makes sharing code simple at first, but add significant complexity as a project developes and grows larger.
+
+For more details and examples of how mixins create problems down-the-line, see these excellent blog posts:
+* [Mixins Considered Harmful](https://reactjs.org/blog/2016/07/13/mixins-considered-harmful.html)
+* [Why Are Mixins Considered Harmful?](http://raganwald.com/2016/07/16/why-are-mixins-considered-harmful.html)
+
+
+```javascript
+// GOOD
+// my-utils.js
+export function isValidClassName(classname) {
+  return !!className.match('-class');
+}
+
+export function hideModal(obj, value) {
+  set(obj, 'isHidden', value);
+}
+
+// my-component.js
+import { isValidClassName } from 'my-utils';
+
+export default Component.extend({
+  aComputedProperty: computed('obj', function() {
+    return isValidClassName(get(obj, 'className'));
+  }),
+});
+
+===========================================================
+
+// BAD
+// my-mixin.js
+export default Mixin.create({
+  isValidClassName(classname) {
+    return !!className.match('-class');
+  },
+
+  hideModal(value) {
+    this.set('isHidden', value);
+  }
+});
+
+// my-component.js
+import myMixin from 'my-mixin';
+
+export default Component.extend(myMixin, {
+  aComputedProperty: computed('obj', function() {
+    return this.isValidClassName(get(obj, 'className'));
+  }),
+});
+```
diff --git a/lib/rules/no-new-mixins.js b/lib/rules/no-new-mixins.js
new file mode 100644
index 00000000..f76a47d3
--- /dev/null
+++ b/lib/rules/no-new-mixins.js
@@ -0,0 +1,31 @@
+'use strict';
+
+const ember = require('../utils/ember');
+
+//------------------------------------------------------------------------------
+// General rule - Don't create new mixins
+//------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Prevents creation of new mixins',
+      category: 'Best Practices',
+      recommended: false
+    },
+    fixable: null, // or "code" or "whitespace"
+  },
+
+  create(context) {
+    const message = "Don't create new mixins";
+    const filePath = context.getFilename();
+
+    return {
+      CallExpression(node) {
+        if (ember.isEmberMixin(node, filePath)) {
+          context.report(node, message);
+        }
+      },
+    };
+  }
+};


Test patch:
diff --git a/tests/lib/rules/no-new-mixins.js b/tests/lib/rules/no-new-mixins.js
new file mode 100644
index 00000000..66f675d3
--- /dev/null
+++ b/tests/lib/rules/no-new-mixins.js
@@ -0,0 +1,50 @@
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-new-mixins');
+const RuleTester = require('eslint').RuleTester;
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const MESSAGE = rule.meta.message;
+const eslintTester = new RuleTester({
+  parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+});
+eslintTester.run('no-new-mixins', rule, {
+  valid: [
+    {
+      code: `
+        import mixin from "some/addon";
+        export default mixin;
+      `,
+    },
+    {
+      code: 'export default mixin.create({actions: {},});',
+    },
+  ],
+  invalid: [
+    {
+      code: `
+        import Ember from "ember";
+
+        export default Ember.Mixin.create({});
+      `,
+      errors: [{
+        message: MESSAGE,
+      }],
+    },
+    {
+      code: `
+        import Mixin from "@ember/object/mixin";
+
+        export default Mixin.create({});
+      `,
+      errors: [{
+        message: MESSAGE,
+      }],
+    },
+  ],
+});