Eslint-plugin-react: jsx-sort-props regression breaks callbacksLast option.

[phyllisstein]

I noticed after upgrading to v7.0.0 of this package that the jsx-sort-props rule is no longer honoring my callbacksLast flag. Given this hunk of my ESLint configuration...

  react/jsx-sort-props:
    - warn
    -
      callbacksLast: true
      ignoreCase: true
      reservedFirst: false
      shorthandFirst: true

...a line like this...

<AppBar showLogo={ showLogo } onLogoClick={ this.handleNavigateHome } onToggleMenu={ this.handleToggleMenu } />

...will yield the following warnings:

warning: Props should be sorted alphabetically (react/jsx-sort-props) at src/app/views/app/index.jsx:118:39:
  116 |           <AppMenu />
  117 |         </Popover>
> 118 |         <AppBar showLogo={ showLogo } onLogoClick={ this.handleNavigateHome } onToggleMenu={ this.handleToggleMenu } />
      |                                       ^
  119 |         <main className={ styles.main }>
  120 |           <div className={ styles.mainWrapper } ref={ node => this.scroller = node }>
  121 |             <Switch>


warning: Props should be sorted alphabetically (react/jsx-sort-props) at src/app/views/app/index.jsx:118:79:
  116 |           <AppMenu />
  117 |         </Popover>
> 118 |         <AppBar showLogo={ showLogo } onLogoClick={ this.handleNavigateHome } onToggleMenu={ this.handleToggleMenu } />
      |                                                                               ^
  119 |         <main className={ styles.main }>
  120 |           <div className={ styles.mainWrapper } ref={ node => this.scroller = node }>
  121 |             <Switch>

My expectation would be that with callbacksLast enabled, the showLogo prop should appear before the on... props, which latter should be sorted alphabetically at the end into onLogoClick then onToggleMenu. Though I could certainly be misunderstanding some change to the rule.

Thanks in advance for your help, and let me know if I can furnish any more debugging information!

[jseminck]

The problem seems to be the combination of ignoreCase: true and callbacksLast: true

callbacksLast is looking for a pattern: on[A-Z]:

function isCallbackPropName(name) {
  return /^on[A-Z]/.test(name);
}

However, ignoreCase: true is calling toLowerCase():

          if (ignoreCase) {
            previousPropName = previousPropName.toLowerCase();
            currentPropName = currentPropName.toLowerCase();
          }

          ... other code ....

           if (callbacksLast) {
              var previousIsCallback = isCallbackPropName(previousPropName);
              var currentIsCallback = isCallbackPropName(currentPropName);
           }

The bug was introduced in 7.0.0. Specifically here - where the check for the isCallbackPropName() was moved from before if (ignoreCase) to after this condition.

https://github.com/yannickcr/eslint-plugin-react/pull/1134/commits/6e18e40896ab3d22be5376675affad240ea4d4f9#diff-bf26edcfae7aa27411d9a791b4eb8a8eL74

Seems like a simple fix, moving it back to it's original position does not break any tests. Will create a PR that includes a test case containing these two properties as defined options.