Skip to content

Commit

Permalink
fix: Rework the propNameSorter to be less dependents of node sort int…
Browse files Browse the repository at this point in the history
…ernals
  • Loading branch information
armandabric committed May 12, 2019
1 parent 105cee4 commit a9ee312
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 52 deletions.
4 changes: 2 additions & 2 deletions src/formatter/formatReactElementNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import spacer from './spacer';
import formatTreeNode from './formatTreeNode';
import formatProp from './formatProp';
import mergeSiblingPlainStringChildrenReducer from './mergeSiblingPlainStringChildrenReducer';
import propNameSorter from './propNameSorter';
import sortPropsByNames from './sortPropsByNames';
import type { Options } from './../options';
import type { ReactElementTreeNode } from './../tree';

Expand Down Expand Up @@ -137,7 +137,7 @@ export default (
.filter(defaultPropName => !visibleAttributeNames.includes(defaultPropName))
.forEach(defaultPropName => visibleAttributeNames.push(defaultPropName));

const attributes = visibleAttributeNames.sort(propNameSorter(sortProps));
const attributes = sortPropsByNames(sortProps)(visibleAttributeNames);

attributes.forEach(attributeName => {
const {
Expand Down
23 changes: 0 additions & 23 deletions src/formatter/propNameSorter.js

This file was deleted.

27 changes: 0 additions & 27 deletions src/formatter/propNameSorter.spec.js

This file was deleted.

26 changes: 26 additions & 0 deletions src/formatter/sortPropsByNames.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* @flow */

const isKeyOrRefProps = (propName: string) => ['key', 'ref'].includes(propName);

export default (shouldSortUserProps: boolean) => (
props: string[]
): string[] => {
const haveKeyProp = props.includes('key');
const haveRefProp = props.includes('ref');

const userPropsOnly = props.filter(oneProp => !isKeyOrRefProps(oneProp));

const sortedProps = shouldSortUserProps
? [...userPropsOnly.sort()] // We use basic lexical order
: [...userPropsOnly];

if (haveRefProp) {
sortedProps.unshift('ref');
}

if (haveKeyProp) {
sortedProps.unshift('key');
}

return sortedProps;
};
27 changes: 27 additions & 0 deletions src/formatter/sortPropsByNames.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* @flow */

import sortPropsByNames from './sortPropsByNames';

test('sortPropsByNames should always move the `key` and `ref` keys first', () => {
const fixtures = ['c', 'key', 'a', 'ref', 'b'];

expect(sortPropsByNames(false)(fixtures)).toEqual([
'key',
'ref',
'c',
'a',
'b',
]);
});

test('sortPropsByNames should always sort the props and keep `key` and `ref` keys first', () => {
const fixtures = ['c', 'key', 'a', 'ref', 'b'];

expect(sortPropsByNames(true)(fixtures)).toEqual([
'key',
'ref',
'a',
'b',
'c',
]);
});

0 comments on commit a9ee312

Please sign in to comment.