Eslint-plugin-react: Add option to ignore spread operator in sort-default-props

Created on 27 Feb 2019  路  6Comments  路  Source: yannickcr/eslint-plugin-react

With foo = { bar: baz }.

{ ...foo, bar } = { bar }

And

{ bar, ...for } = { baz }

So I think, an option to ignore spread operator in sort default props will be great.
Correct example

{ a: 'a', c: 'c', ...foo, b: 'b', d: 'd' }

Incorrect example

{ a: 'a', ...foo, c: 'c', b: 'b', d: 'd' }

Plus, it allow to develop a fixer (asked here) without breaking the code.

enhancement help wanted rule

All 6 comments

I very much don鈥檛 agree that this would be good. A correct fixer will never move anything across a spread boundary, and while it鈥檚 fine to use linting rules to block language syntax, using linting rules to willfully break language syntax is not ok.

@ljharb I'm not sure we understand each other.
Why are you talking about willfuly break language syntax ?

I was aking about an option to avoid having error in case like this

bar.defaultProps = { ...foo,  anotherProps }

Cause the rule actually ask for

bar.defaultProps = { anotherProps, ...foo,  }

It's not a good thing if eslint ask for breaking my code and I have to add disable-next-line multiple times in my project.

Gotcha, that makes sense. Thanks for clarifying.

An option to only enforce sorting within spread boundaries seems like a good addition.

To be clear with everyone

Without the option

{ a, bar, foo } // valid
{ a, foo, bar } // invalid
{ ...foo, a, bar } // invalid
{ a, ...foo, bar } // invalid
{ a, bar, ...foo } // valid
{ bar, a, ...foo } // invalid

With the option

{ a, bar, foo } // valid
{ a, foo, bar } // invalid
{ ...foo, a, bar } // valid
{ a, ...foo, bar } // valid
{ a, bar, ...foo } // valid
{ bar, a, ...foo } // invalid

@ljharb I started to look into the code to implement the option.

In test, we can see

{
    code: [
      'export default class ClassWithSpreadInPropTypes extends BaseClass {',
      '  static propTypes = {',
      '    b: PropTypes.string,',
      '    ...c.propTypes,',
      '    a: PropTypes.string',
      '  }',
      '  static defaultProps = {',
      '    b: "b",',
      '    ...c.defaultProps,',
      '    a: "a"',
      '  }',
      '}'
    ].join('\n'),
    parser: 'babel-eslint'
  }

Is expected to be a valid code. There is a check

if (/SpreadProperty$/.test(curr.type)) {
          return decls[idx + 1];
        }

So I think I found a bug. Cause this test case return an error.

{
    code: [
      'const defaults = {',
      '  b: "b"',
      '};',
      'const First = (props) => <div />;',
      'export const propTypes = {',
      '    a: PropTypes.any,',
      '    z: PropTypes.string,',
      '};',
      'export const defaultProps = {',
      '    ...defaults,',
      '    a: "a",',
      '    z: "z",',
      '};',
      'First.propTypes = propTypes;',
      'First.defaultProps = defaultProps;'
    ].join('\n')
  }

@ljharb I found the fix and made the PR

if (/SpreadProperty$/.test(curr.type) || /SpreadElement$/.test(curr.type)) {
          return decls[idx + 1];
        }
Was this page helpful?
0 / 5 - 0 ratings