Skip to content

Commit

Permalink
Merge pull request ember-cli#256 from nlfurniss/no-mixins
Browse files Browse the repository at this point in the history
Add rule to disallow mixins
  • Loading branch information
rwjblue authored May 25, 2018
2 parents a8a1a32 + e34cf59 commit 04c7696
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 0 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ node_modules
coverage
npm-debug.log
*.swp
.vscode
54 changes: 54 additions & 0 deletions docs/rules/no-new-mixins.md
Original file line number Diff line number Diff line change
@@ -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'));
}),
});
```
31 changes: 31 additions & 0 deletions lib/rules/no-new-mixins.js
Original file line number Diff line number Diff line change
@@ -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);
}
},
};
}
};
50 changes: 50 additions & 0 deletions tests/lib/rules/no-new-mixins.js
Original file line number Diff line number Diff line change
@@ -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,
}],
},
],
});

0 comments on commit 04c7696

Please sign in to comment.