From a3428f6d7ff535fe24e5e2ff064239a1e2b73fee Mon Sep 17 00:00:00 2001 From: ametreniuc Date: Wed, 29 Aug 2018 16:37:10 +0300 Subject: [PATCH 1/3] Group static lifecycle methods under the lifecycle group --- lib/rules/sort-comp.js | 46 +++++++++++++----------------------- tests/lib/rules/sort-comp.js | 10 +------- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 4e2602a74e..8492249497 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -75,6 +75,20 @@ function getMethodsOrder(userConfig) { return config; } +/** + * Returns a function that check if the method name is a lifecycle method + * @param {Object} userConfig The user configuration. + * @returns {Function} isLifecycle + */ +function makeIsLifecycle(userConfig) { + userConfig = userConfig || {}; + const groups = util._extend(defaultConfig.groups, userConfig.groups); + const lifecycleGroup = groups.lifecycle || []; + return function isLifecycle(methodName) { + return arrayIncludes(lifecycleGroup, methodName); + }; +} + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -119,6 +133,7 @@ module.exports = { const MISPOSITION_MESSAGE = '{{propA}} should be placed {{position}} {{propB}}'; const methodsOrder = getMethodsOrder(context.options[0]); + const isLifecycle = makeIsLifecycle(context.options[0]); // -------------------------------------------------------------------------- // Public @@ -148,7 +163,7 @@ module.exports = { methodGroupIndexes.push(groupIndex); } } else if (currentGroup === 'static-methods') { - if (method.static) { + if (method.static && !isLifecycle(method.name)) { methodGroupIndexes.push(groupIndex); } } else if (currentGroup === 'instance-variables') { @@ -159,34 +174,7 @@ module.exports = { if (method.instanceMethod) { methodGroupIndexes.push(groupIndex); } - } else if (arrayIncludes([ - 'displayName', - 'propTypes', - 'contextTypes', - 'childContextTypes', - 'mixins', - 'statics', - 'defaultProps', - 'constructor', - 'getDefaultProps', - 'state', - 'getInitialState', - 'getChildContext', - 'getDerivedStateFromProps', - 'componentWillMount', - 'UNSAFE_componentWillMount', - 'componentDidMount', - 'componentWillReceiveProps', - 'UNSAFE_componentWillReceiveProps', - 'shouldComponentUpdate', - 'componentWillUpdate', - 'UNSAFE_componentWillUpdate', - 'getSnapshotBeforeUpdate', - 'componentDidUpdate', - 'componentDidCatch', - 'componentWillUnmount', - 'render' - ], currentGroup)) { + } else if (isLifecycle(currentGroup) || currentGroup === 'render') { if (currentGroup === method.name) { methodGroupIndexes.push(groupIndex); } diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 8277b10ab3..77e1434a20 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -468,15 +468,7 @@ ruleTester.run('sort-comp', rule, { }] }, { code: [ - '// static lifecycle methods can be grouped (with statics)', - 'class Hello extends React.Component {', - ' static getDerivedStateFromProps() {}', - ' constructor() {}', - '}' - ].join('\n') - }, { - code: [ - '// static lifecycle methods can be grouped (with lifecycle)', + '// static lifecycle methods should be grouped with lifecycle', 'class Hello extends React.Component {', ' constructor() {}', ' static getDerivedStateFromProps() {}', From a3a76dde03a014cbfebceb85a5f47cad177dbe50 Mon Sep 17 00:00:00 2001 From: ametreniuc Date: Wed, 29 Aug 2018 17:25:19 +0300 Subject: [PATCH 2/3] Fix typo --- lib/rules/sort-comp.js | 2 +- tests/lib/rules/sort-comp.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index 8492249497..f505446074 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -76,7 +76,7 @@ function getMethodsOrder(userConfig) { } /** - * Returns a function that check if the method name is a lifecycle method + * Returns a function that checks if the method name is a lifecycle method * @param {Object} userConfig The user configuration. * @returns {Function} isLifecycle */ diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 77e1434a20..2034899cd9 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -468,7 +468,7 @@ ruleTester.run('sort-comp', rule, { }] }, { code: [ - '// static lifecycle methods should be grouped with lifecycle', + '// static lifecycle methods should be grouped under lifecycle', 'class Hello extends React.Component {', ' constructor() {}', ' static getDerivedStateFromProps() {}', From 852f0e39279853ffaf6a4d48092c81588a1e3136 Mon Sep 17 00:00:00 2001 From: ametreniuc Date: Wed, 19 Sep 2018 01:24:57 +0300 Subject: [PATCH 3/3] Add preferLifecycle option --- lib/rules/sort-comp.js | 22 ++++++++++++++-------- tests/lib/rules/sort-comp.js | 23 +++++++++++++++++++++-- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/lib/rules/sort-comp.js b/lib/rules/sort-comp.js index f505446074..f788f18993 100644 --- a/lib/rules/sort-comp.js +++ b/lib/rules/sort-comp.js @@ -47,7 +47,8 @@ const defaultConfig = { 'componentDidCatch', 'componentWillUnmount' ] - } + }, + preferLifecycle: false }; /** @@ -56,8 +57,6 @@ const defaultConfig = { * @returns {Array} Methods order */ function getMethodsOrder(userConfig) { - userConfig = userConfig || {}; - const groups = util._extend(defaultConfig.groups, userConfig.groups); const order = userConfig.order || defaultConfig.order; @@ -81,7 +80,6 @@ function getMethodsOrder(userConfig) { * @returns {Function} isLifecycle */ function makeIsLifecycle(userConfig) { - userConfig = userConfig || {}; const groups = util._extend(defaultConfig.groups, userConfig.groups); const lifecycleGroup = groups.lifecycle || []; return function isLifecycle(methodName) { @@ -121,6 +119,10 @@ module.exports = { } } } + }, + preferLifecycle: { + type: 'boolean', + default: defaultConfig.default } }, additionalProperties: false @@ -132,8 +134,10 @@ module.exports = { const MISPOSITION_MESSAGE = '{{propA}} should be placed {{position}} {{propB}}'; - const methodsOrder = getMethodsOrder(context.options[0]); - const isLifecycle = makeIsLifecycle(context.options[0]); + const userConfig = context.options[0] || {}; + const methodsOrder = getMethodsOrder(userConfig); + const isLifecycle = makeIsLifecycle(userConfig); + const preferLifecycle = userConfig.preferLifecycle || defaultConfig.preferLifecycle; // -------------------------------------------------------------------------- // Public @@ -163,8 +167,10 @@ module.exports = { methodGroupIndexes.push(groupIndex); } } else if (currentGroup === 'static-methods') { - if (method.static && !isLifecycle(method.name)) { - methodGroupIndexes.push(groupIndex); + if (method.static) { + if (!(preferLifecycle && isLifecycle(method.name))) { + methodGroupIndexes.push(groupIndex); + } } } else if (currentGroup === 'instance-variables') { if (method.instanceVariable) { diff --git a/tests/lib/rules/sort-comp.js b/tests/lib/rules/sort-comp.js index 2034899cd9..59f9c89412 100644 --- a/tests/lib/rules/sort-comp.js +++ b/tests/lib/rules/sort-comp.js @@ -468,12 +468,21 @@ ruleTester.run('sort-comp', rule, { }] }, { code: [ - '// static lifecycle methods should be grouped under lifecycle', + '// by default static lifecycle methods can be grouped with static', 'class Hello extends React.Component {', - ' constructor() {}', ' static getDerivedStateFromProps() {}', + ' constructor() {}', '}' ].join('\n') + }, { + code: [ + '// static lifecycle methods should be grouped under lifecycle with the preferLifecycle set to true', + 'class Hello extends React.Component {', + ' constructor() {}', + ' static getDerivedStateFromProps() {}', + '}' + ].join('\n'), + options: [{preferLifecycle: true}] }], invalid: [{ @@ -758,5 +767,15 @@ ruleTester.run('sort-comp', rule, { 'render' ] }] + }, { + code: [ + '// static lifecycle methods should warn if not grouped under the lifecycle group', + 'class Hello extends React.Component {', + ' static getDerivedStateFromProps() {}', + ' constructor() {}', + '}' + ].join('\n'), + errors: [{message: 'getDerivedStateFromProps should be placed after constructor'}], + options: [{preferLifecycle: true}] }] });