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

react/function-component-definition: False positive on Redux mapStateToProps function #3057

Closed
bhollis opened this issue Aug 29, 2021 · 10 comments · Fixed by #3355
Closed

react/function-component-definition: False positive on Redux mapStateToProps function #3057

bhollis opened this issue Aug 29, 2021 · 10 comments · Fixed by #3355

Comments

@bhollis
Copy link

bhollis commented Aug 29, 2021

After upgrading to 7.25.0 I get a false positive from react/function-component-definition on the anonymous arrow function returned inside this:

function mapStateToProps() {
  const internItems = makeInternArray<DimItem>();
  const internClassList = makeInternArray<DestinyClass>();

  return (
    state: RootState,
    props: ProvidedProps
  ): StoreProps & {
    store: DimStore | null;
  } => {
    const { store, bucket, singleCharacter } = props;

    let items = findItemsByBucket(store, bucket.hash);
    if (singleCharacter && store.isVault && bucket.vaultBucket) {
      for (const otherStore of storesSelector(state)) {
        if (!otherStore.current && !otherStore.isVault) {
          items = [...items, ...findItemsByBucket(otherStore, bucket.hash)];
        }
      }
      const currentStore = currentStoreSelector(state)!;
      items = items.filter(
        (i) => i.classType === DestinyClass.Unknown || i.classType === currentStore.classType
      );
    }

    return {
      store: null,
      destinyVersion: store.destinyVersion,
      storeId: store.id,
      storeName: store.name,
      storeClassType: store.classType,
      isVault: store.isVault,
      items: internItems(items),
      itemSortOrder: itemSortOrderSelector(state),
      // We only need this property when this is a vault armor bucket
      storeClassList:
        store.isVault && bucket.inArmor
          ? internClassList(sortedStoresSelector(state).map((s) => s.classType))
          : emptyArray(),
      characterOrder: characterOrderSelector(state),
      isPhonePortrait: isPhonePortraitSelector(state),
    };
  };
}

Since this is returning an object and not JSX, I don't think it should be detected as a React component.

@ljharb
Copy link
Member

ljharb commented Aug 30, 2021

This may be related to #3054.

@bhollis
Copy link
Author

bhollis commented Sep 1, 2021

This is fixed in 7.25.1

@bhollis bhollis closed this as completed Sep 1, 2021
@bhollis bhollis reopened this Sep 1, 2021
@bhollis
Copy link
Author

bhollis commented Sep 1, 2021

Actually I spoke a touch too soon - it's no longer a violation but autofix now rewrites the arrow function to a regular function because I have react/function-component-definition enabled.

@ljharb
Copy link
Member

ljharb commented Sep 1, 2021

Presumably that means it is still a violation, it's just that the autofix makes it not be one?

@bhollis
Copy link
Author

bhollis commented Sep 1, 2021

That makes sense. I suspect the categorization changed a bit so that it now hits a different rule that has an autofix.

@mithodin
Copy link

mithodin commented Apr 19, 2022

This also gets incorrectly detected as a React component:

export default (key, subTree = {}) =>
    (state) => {
        const dataInStore = getFromDataModel(key)(state);
        const fullPaths = dataInStore.map((item, index) => {
            return [key, index];
        });

        return {
            key,
            paths: fullPaths.map((p) => [p[1]]),
            fullPaths,
            subTree: Object.keys(subTree).length ? subTree : null,
        };
    };

Version 7.28.0 and 7.29.4

@GiancarlosIO
Copy link

👀

@TildaDares
Copy link
Contributor

@ljharb I'd like to work on this.

@TildaDares
Copy link
Contributor

If I understand this correctly, the function-component-definition rule should only work on React components specifically ones that return JSX.

@ljharb
Copy link
Member

ljharb commented Aug 4, 2022

@TildaDares yes, this seems to be related to component detection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants