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.

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

[Fix] sort props test cases #3452

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,13 +14,15 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
* configs: avoid legacy config system error ([#3461][] @ljharb)
* [`jsx-no-target-blank`]: allow ternaries with literals ([#3464][] @akulsr0)
* [`sort-prop-types`]: restore autofixing ([#2574][] @ROSSROSALES)

### Changed
* [Perf] component detection: improve performance by avoiding traversing parents unnecessarily ([#3459][] @golopot)

[#3464]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3464
[#3461]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3461
[#3459]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3459
[#3452]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3452
[#3449]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3449
[#3424]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3429
[#2848]: https://github.com/jsx-eslint/eslint-plugin-react/pull/2848
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -342,7 +342,7 @@ module.exports = [
| | 🔧 | | [react/self-closing-comp](docs/rules/self-closing-comp.md) | Disallow extra closing tags for components without children |
| | | | [react/sort-comp](docs/rules/sort-comp.md) | Enforce component methods order |
| | | | [react/sort-default-props](docs/rules/sort-default-props.md) | Enforce defaultProps declarations alphabetical sorting |
| | | | [react/sort-prop-types](docs/rules/sort-prop-types.md) | Enforce propTypes declarations alphabetical sorting |
| | 🔧 | | [react/sort-prop-types](docs/rules/sort-prop-types.md) | Enforce propTypes declarations alphabetical sorting |
| | | | [react/state-in-constructor](docs/rules/state-in-constructor.md) | Enforce class component state initialization style |
| | | | [react/static-property-placement](docs/rules/static-property-placement.md) | Enforces where React component static properties should be positioned. |
| | | | [react/style-prop-object](docs/rules/style-prop-object.md) | Enforce style prop value is an object |
Expand Down
2 changes: 2 additions & 0 deletions docs/rules/sort-prop-types.md
Expand Up @@ -2,6 +2,8 @@

💼 This rule is enabled in the following [configs](https://github.com/jsx-eslint/eslint-plugin-react#shareable-configurations): `all`.

🔧 This rule is automatically fixable using the `--fix` [flag](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) on the command line.

Some developers prefer to sort prop type declarations alphabetically to be able to find necessary declaration easier at the later time. Others feel that it adds complexity and becomes burden to maintain.

## Rule Details
Expand Down
32 changes: 16 additions & 16 deletions lib/rules/sort-prop-types.js
Expand Up @@ -8,7 +8,7 @@ const variableUtil = require('../util/variable');
const propsUtil = require('../util/props');
const docsUrl = require('../util/docsUrl');
const propWrapperUtil = require('../util/propWrapper');
// const propTypesSortUtil = require('../util/propTypesSort');
const propTypesSortUtil = require('../util/propTypesSort');
const report = require('../util/report');

// ------------------------------------------------------------------------------
Expand All @@ -29,7 +29,7 @@ module.exports = {
recommended: false,
url: docsUrl('sort-prop-types'),
},
// fixable: 'code',
fixable: 'code',

messages,

Expand Down Expand Up @@ -106,17 +106,17 @@ module.exports = {
return;
}

// function fix(fixer) {
// return propTypesSortUtil.fixPropTypesSort(
// fixer,
// context,
// declarations,
// ignoreCase,
// requiredFirst,
// callbacksLast,
// sortShapeProp
// );
// }
function fix(fixer) {
return propTypesSortUtil.fixPropTypesSort(
fixer,
context,
declarations,
ignoreCase,
requiredFirst,
callbacksLast,
sortShapeProp
);
}

const callbackPropsLastSeen = new WeakSet();
const requiredPropsFirstSeen = new WeakSet();
Expand Down Expand Up @@ -150,7 +150,7 @@ module.exports = {
requiredPropsFirstSeen.add(curr);
report(context, messages.requiredPropsFirst, 'requiredPropsFirst', {
node: curr,
// fix
fix,
});
}
return curr;
Expand All @@ -168,7 +168,7 @@ module.exports = {
callbackPropsLastSeen.add(prev);
report(context, messages.callbackPropsLast, 'callbackPropsLast', {
node: prev,
// fix
fix,
});
}
return prev;
Expand All @@ -180,7 +180,7 @@ module.exports = {
propsNotSortedSeen.add(curr);
report(context, messages.propsNotSorted, 'propsNotSorted', {
node: curr,
// fix
fix,
});
}
return prev;
Expand Down
42 changes: 38 additions & 4 deletions lib/util/propTypesSort.js
Expand Up @@ -116,9 +116,39 @@ function sorter(a, b, context, ignoreCase, requiredFirst, callbacksLast) {
* @param {Boolean=} sortShapeProp whether or not to sort propTypes defined in PropTypes.shape.
* @returns {Object|*|{range, text}} the sort order of the two elements.
*/
const commentnodeMap = new WeakMap(); // all nodes reference WeakMap for start and end range
function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirst, callbacksLast, sortShapeProp) {
function sortInSource(allNodes, source) {
const originalSource = source;
const sourceCode = context.getSourceCode();
for (let i = 0; i < allNodes.length; i++) {
const node = allNodes[i];
let commentAfter = [];
let commentBefore = [];
try {
commentBefore = sourceCode.getCommentsBefore(node);
commentAfter = sourceCode.getCommentsAfter(node);
} catch (e) { /**/ }
if (commentAfter.length === 0 && commentBefore.length === 0) {
commentnodeMap.set(node, { start: node.range[0], end: node.range[1], hasComment: false });
} else {
const firstCommentBefore = commentBefore[0];
if (commentBefore.length === 1) {
commentnodeMap.set(node, { start: firstCommentBefore.range[0], end: node.range[1], hasComment: true });
}
const firstCommentAfter = commentAfter[0];
if (commentAfter.length === 1) {
commentnodeMap.set(node, { start: node.range[0], end: firstCommentAfter.range[1], hasComment: true });
}
if (commentBefore.length === 1 && commentAfter.length === 1) {
commentnodeMap.set(node, {
start: firstCommentBefore.range[0],
end: firstCommentAfter.range[1],
hasComment: true,
});
}
}
}
const nodeGroups = allNodes.reduce((acc, curr) => {
if (curr.type === 'ExperimentalSpreadProperty' || curr.type === 'SpreadElement') {
acc.push([]);
Expand All @@ -135,7 +165,11 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs

source = nodes.reduceRight((acc, attr, index) => {
const sortedAttr = sortedAttributes[index];
let sortedAttrText = context.getSourceCode().getText(sortedAttr);
const sourceCodeText = sourceCode.getText();
let sortedAttrText = sourceCodeText.substring(
commentnodeMap.get(sortedAttr).start,
commentnodeMap.get(sortedAttr).end
);
if (sortShapeProp && isShapeProp(sortedAttr.value)) {
const shape = getShapeProperties(sortedAttr.value);
if (shape) {
Expand All @@ -146,16 +180,16 @@ function fixPropTypesSort(fixer, context, declarations, ignoreCase, requiredFirs
sortedAttrText = attrSource.slice(sortedAttr.range[0], sortedAttr.range[1]);
}
}
return `${acc.slice(0, attr.range[0])}${sortedAttrText}${acc.slice(attr.range[1])}`;
return `${acc.slice(0, commentnodeMap.get(attr).start)}${sortedAttrText}${acc.slice(commentnodeMap.get(attr).end)}`;
}, source);
});
return source;
}

const source = sortInSource(declarations, context.getSourceCode().getText());

const rangeStart = declarations[0].range[0];
const rangeEnd = declarations[declarations.length - 1].range[1];
const rangeStart = commentnodeMap.get(declarations[0]).start;
const rangeEnd = commentnodeMap.get(declarations[declarations.length - 1]).end;
return fixer.replaceTextRange([rangeStart, rangeEnd], source.slice(rangeStart, rangeEnd));
}

Expand Down