Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Group static lifecycle methods under lifecycle #1962

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Group static lifecycle methods under lifecycle #1962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3428f6
Group static lifecycle methods under the lifecycle group
metreniuk Aug 29, 2018
a3a76dd
Fix typo
metreniuk Aug 29, 2018
852f0e3
Add preferLifecycle option
metreniuk Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 27 additions & 33 deletions lib/rules/sort-comp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ const defaultConfig = {
'componentDidCatch',
'componentWillUnmount'
]
}
},
preferLifecycle: false
};

/**
Expand All @@ -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;

Expand All @@ -75,6 +74,19 @@ function getMethodsOrder(userConfig) {
return config;
}

/**
* Returns a function that checks if the method name is a lifecycle method
* @param {Object} userConfig The user configuration.
* @returns {Function} isLifecycle
*/
function makeIsLifecycle(userConfig) {
const groups = util._extend(defaultConfig.groups, userConfig.groups);
const lifecycleGroup = groups.lifecycle || [];
return function isLifecycle(methodName) {
return arrayIncludes(lifecycleGroup, methodName);
};
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -107,6 +119,10 @@ module.exports = {
}
}
}
},
preferLifecycle: {
type: 'boolean',
default: defaultConfig.default
}
},
additionalProperties: false
Expand All @@ -118,7 +134,10 @@ module.exports = {

const MISPOSITION_MESSAGE = '{{propA}} should be placed {{position}} {{propB}}';

const methodsOrder = getMethodsOrder(context.options[0]);
const userConfig = context.options[0] || {};
const methodsOrder = getMethodsOrder(userConfig);
const isLifecycle = makeIsLifecycle(userConfig);
const preferLifecycle = userConfig.preferLifecycle || defaultConfig.preferLifecycle;

// --------------------------------------------------------------------------
// Public
Expand Down Expand Up @@ -149,7 +168,9 @@ module.exports = {
}
} else if (currentGroup === 'static-methods') {
if (method.static) {
methodGroupIndexes.push(groupIndex);
if (!(preferLifecycle && isLifecycle(method.name))) {
methodGroupIndexes.push(groupIndex);
}
}
} else if (currentGroup === 'instance-variables') {
if (method.instanceVariable) {
Expand All @@ -159,34 +180,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);
}
Expand Down
17 changes: 14 additions & 3 deletions tests/lib/rules/sort-comp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,20 +468,21 @@ ruleTester.run('sort-comp', rule, {
}]
}, {
code: [
'// static lifecycle methods can be grouped (with statics)',
'// by default static lifecycle methods can be grouped with static',
'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 under lifecycle with the preferLifecycle set to true',
'class Hello extends React.Component {',
' constructor() {}',
' static getDerivedStateFromProps() {}',
'}'
].join('\n')
].join('\n'),
options: [{preferLifecycle: true}]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of modifying this test, can we duplicate it and modify the duplicate?

}],

invalid: [{
Expand Down Expand Up @@ -766,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}]
}]
});