Skip to content

Commit

Permalink
feat: enable exhaustive-deps, but ignore all current errors (#2244)
Browse files Browse the repository at this point in the history
### 🎯 Goal

Generally, `react-hooks/exhaustive-deps` eslint rule is very useful, but
for historical reasons it's not always followed in our SDK. We can't fix
every place this rule is violated, but at least we want to have
exhaustive deps moving forward.

This PR enable this eslint rule, but adds in-place ignores everywhere
it's currently violated. That way we can make fixes incrementally, and
benefit from improved linting moving forward.

### 🛠 Implementation details

All changes were made automatically (and manually reviewed afterwards):

1. Enable `react-hooks/exhaustive-deps`
2. Export all linting errors to `lint.json`:

```sh
npx eslint 'src/**/*.{js,ts,tsx,md}' -f json -o lint.json 
```

3. Use this script to add in-place ignores (adapted from
https://stackoverflow.com/questions/60629026/is-there-a-way-to-programmatically-suppress-all-eslint-errors-on-a-line-by-line):

```js
const json = require('./lint.json');
const fs = require('fs');

json.forEach(({ filePath, messages, source }) => {
  if (!source) {
    return;
  }

  const data = source.split('\n');
  let offset = 1;

  const groupedMessages = messages.reduce((acc, next) => {
    const prevMessages = acc[next.line] ? acc[next.line] : [];
    const duplicateRuleForLine = prevMessages.find((message) => message.ruleId === next.ruleId);
    const applicableRule = next.ruleId === 'react-hooks/exhaustive-deps';

    if (duplicateRuleForLine || !applicableRule) {
      return acc;
    }

    return {
      ...acc,
      [next.line]: [...prevMessages, next],
    };
  }, {});

  Object.entries(groupedMessages).forEach(([line, messages]) => {
    const ignore = `// eslint-disable-next-line ${messages.map(({ ruleId }) => ruleId).join(' ')}`;
    data.splice(line - offset, 0, ignore);
    offset--;
  });

  const updated = data.join('\n');

  fs.writeFile(filePath, updated, function (err) {
    if (err) return console.log(err);
  });
});
```
  • Loading branch information
myandrienko authored Jan 18, 2024
1 parent 651d3e7 commit d07861f
Show file tree
Hide file tree
Showing 63 changed files with 89 additions and 2 deletions.
4 changes: 2 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"max-classes-per-file": 0,
"camelcase": 0,
"react-hooks/rules-of-hooks": 1,
"react-hooks/exhaustive-deps": 0,
"react-hooks/exhaustive-deps": 1,
"jest/prefer-inline-snapshots": 0,
"jest/lowercase-name": 0,
"jest/prefer-expect-assertions": 0,
Expand Down Expand Up @@ -166,7 +166,7 @@
"@typescript-eslint/ban-ts-comment": 0,
"@typescript-eslint/no-unused-vars": 1,
"@typescript-eslint/no-var-requires": 0,
"react-hooks/exhaustive-deps": 0,
"react-hooks/exhaustive-deps": 1,
"react-native/no-inline-styles": 0,
"array-callback-return": 2,
"arrow-body-style": 2,
Expand Down
1 change: 1 addition & 0 deletions src/components/Attachment/Attachment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const Attachment = <
) => {
const { attachments } = props;

// eslint-disable-next-line react-hooks/exhaustive-deps
const groupedAttachments = useMemo(() => renderGroupedAttachments(props), [attachments]);

return (
Expand Down
1 change: 1 addition & 0 deletions src/components/Attachment/AttachmentContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ export const MediaContainer = <
);
setAttachmentConfiguration(config);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [videoElement, videoAttachmentSizeHandler, attachment]);

const content = (
Expand Down
1 change: 1 addition & 0 deletions src/components/Attachment/hooks/useAudioController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const useAudioController = () => {
audioRef.current.play();

return () => {
// eslint-disable-next-line react-hooks/exhaustive-deps
audioRef.current?.pause();

window.clearInterval(interval);
Expand Down
3 changes: 3 additions & 0 deletions src/components/Channel/Channel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ const ChannelInner = <
client.off('user.deleted', handleEvent);
notificationTimeouts.forEach(clearTimeout);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
channel.cid,
channelQueryOptions,
Expand Down Expand Up @@ -987,6 +988,7 @@ const ChannelInner = <
skipMessageDataMemoization,
updateMessage,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[
channel.cid,
deleteMessage,
Expand Down Expand Up @@ -1046,6 +1048,7 @@ const ChannelInner = <
TypingIndicator: props.TypingIndicator,
VirtualMessage: props.VirtualMessage,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.reactionOptions],
);

Expand Down
1 change: 1 addition & 0 deletions src/components/Channel/__tests__/Channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const CallbackEffectWithChannelContexts = ({ callback }) => {
const channelActionContext = useChannelActionContext();
const componentContext = useComponentContext();

// eslint-disable-next-line react-hooks/exhaustive-deps
const channelContext = {
...channelStateContext,
...channelActionContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export const useCreateChannelStateContext = <
watcherCount,
watchers,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[
channelId,
debounceURLEnrichmentMs,
Expand Down
1 change: 1 addition & 0 deletions src/components/Channel/hooks/useCreateTypingContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const useCreateTypingContext = <
() => ({
typing,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[typingValue],
);

Expand Down
3 changes: 3 additions & 0 deletions src/components/ChannelList/ChannelList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,13 @@ const UnMemoizedChannelList = <
setSearchActive(true);
}
additionalChannelSearchProps?.onSearch?.(event);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const onSearchExit = useCallback(() => {
setSearchActive(false);
additionalChannelSearchProps?.onSearchExit?.();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const { channels, hasNextPage, loadNextPage, setChannels } = usePaginatedChannels(
Expand Down Expand Up @@ -317,6 +319,7 @@ const UnMemoizedChannelList = <
client.off('channel.deleted', handleEvent);
client.off('channel.hidden', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [channel?.cid]);

const renderChannel = (item: Channel<StreamChatGenerics>) => {
Expand Down
1 change: 1 addition & 0 deletions src/components/ChannelList/__tests__/ChannelList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,7 @@ describe('ChannelList', () => {
const { channels, setChannels } = useChannelListContext();
useEffect(() => {
setChannelsFromOutside = setChannels;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
return <div>{channelsToIdString(channels)}</div>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ export const useChannelDeletedListener = <
return () => {
client.off('channel.deleted', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@ export const useChannelHiddenListener = <
return () => {
client.off('channel.hidden', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@ export const useChannelTruncatedListener = <
return () => {
client.off('channel.truncated', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ export const useChannelUpdatedListener = <
return () => {
client.off('channel.updated', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,6 @@ export const useChannelVisibleListener = <
return () => {
client.off('channel.visible', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ export const useConnectionRecoveredListener = <
return () => {
client.off('connection.recovered', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
};
1 change: 1 addition & 0 deletions src/components/ChannelList/hooks/useMessageNewListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ export const useMessageNewListener = <
return () => {
client.off('message.new', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [lockChannelOrder]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ export const useNotificationAddedToChannelListener = <
return () => {
client.off('notification.added_to_channel', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ export const useNotificationMessageNewListener = <
return () => {
client.off('notification.message_new', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ export const useNotificationRemovedFromChannelListener = <
return () => {
client.off('notification.removed_from_channel', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [customHandler]);
};
2 changes: 2 additions & 0 deletions src/components/ChannelList/hooks/usePaginatedChannels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const usePaginatedChannels = <
const filterString = useMemo(() => JSON.stringify(filters), [filters]);
const sortString = useMemo(() => JSON.stringify(sort), [sort]);

// eslint-disable-next-line react-hooks/exhaustive-deps
const queryChannels = async (queryType?: string) => {
setError(null);

Expand Down Expand Up @@ -113,6 +114,7 @@ export const usePaginatedChannels = <

useEffect(() => {
queryChannels('reload');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [filterString, sortString]);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@ export const useUserPresenceChangedListener = <
return () => {
client.off('user.presence.changed', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
};
2 changes: 2 additions & 0 deletions src/components/ChannelPreview/ChannelPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const ChannelPreview = <

client.on('notification.mark_read', handleEvent);
return () => client.off('notification.mark_read', handleEvent);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const refreshUnreadCount = useCallback(() => {
Expand Down Expand Up @@ -119,6 +120,7 @@ export const ChannelPreview = <
channel.off('message.updated', handleEvent);
channel.off('message.deleted', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [refreshUnreadCount, channelUpdateCount]);

if (!Preview) return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const useChannelPreviewInfo = <
return () => {
client.off('user.updated', handleEvent);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return {
Expand Down
1 change: 1 addition & 0 deletions src/components/ChannelPreview/hooks/useIsChannelMuted.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const useIsChannelMuted = <

client.on('notification.channel_mutes_updated', handleEvent);
return () => client.off('notification.channel_mutes_updated', handleEvent);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [muted]);

return muted;
Expand Down
2 changes: 2 additions & 0 deletions src/components/ChannelSearch/SearchBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,13 @@ export const SearchBar = (props: SearchBarProps) => {
props.inputRef.current?.removeEventListener('focus', handleFocus);
props.inputRef.current?.addEventListener('blur', handleBlur);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const handleClearClick = useCallback(() => {
exitSearch();
inputProps.inputRef.current?.focus();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const closeAppMenu = useCallback(() => setMenuIsOpen(false), []);
Expand Down
1 change: 1 addition & 0 deletions src/components/ChannelSearch/SearchResults.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ export const SearchResults = <
}
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[focusedResult],
);

Expand Down
5 changes: 5 additions & 0 deletions src/components/ChannelSearch/hooks/useChannelSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export const useChannelSearch = <

document.addEventListener('click', clickListener);
return () => document.removeEventListener('click', clickListener);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [disabled, inputIsFocused, query, exitSearch, clearSearchOnClickOutside]);

useEffect(() => {
Expand All @@ -172,8 +173,10 @@ export const useChannelSearch = <
inputRef.current.addEventListener('keydown', handleKeyDown);

return () => {
// eslint-disable-next-line react-hooks/exhaustive-deps
inputRef.current?.removeEventListener('keydown', handleKeyDown);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [disabled]);

const selectResult = useCallback(
Expand Down Expand Up @@ -206,6 +209,7 @@ export const useChannelSearch = <
exitSearch();
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[clearSearchOnClickOutside, client, exitSearch, onSelectResult, setActiveChannel, setChannels],
);

Expand Down Expand Up @@ -264,6 +268,7 @@ export const useChannelSearch = <
[client, searchForChannels, searchQueryParams],
);

// eslint-disable-next-line react-hooks/exhaustive-deps
const scheduleGetChannels = useCallback(debounce(getChannels, searchDebounceIntervalMs), [
getChannels,
searchDebounceIntervalMs,
Expand Down
2 changes: 2 additions & 0 deletions src/components/Chat/hooks/useChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const useChat = <

client.on('notification.mutes_updated', handleEvent);
return () => client.off('notification.mutes_updated', handleEvent);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [clientMutes?.length]);

useEffect(() => {
Expand All @@ -97,6 +98,7 @@ export const useChat = <
userLanguage: userLanguage || defaultLanguage,
});
});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [i18nInstance]);

const setActiveChannel = useCallback(
Expand Down
1 change: 1 addition & 0 deletions src/components/Chat/hooks/useCreateChatContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const useCreateChatContext = <
themeVersion,
useImageFlagEmojisOnWindows,
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[
channelCid,
channelsQueryError,
Expand Down
1 change: 1 addition & 0 deletions src/components/Gallery/ModalGallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const ModalGallery = <
source: imageSrc,
};
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[images],
);

Expand Down
1 change: 1 addition & 0 deletions src/components/InfiniteScrollPaginator/InfiniteScroll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const InfiniteScroll = (props: PropsWithChildren<InfiniteScrollProps>) =>
],
'InfiniteScroll',
);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useLayoutEffect(() => {
Expand Down
1 change: 1 addition & 0 deletions src/components/LoadMore/LoadMoreButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const UnMemoizedLoadMoreButton = ({

useEffect(() => {
deprecationAndReplacementWarning([[{ refreshing }, { isLoading }]], 'LoadMoreButton');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return (
Expand Down
1 change: 1 addition & 0 deletions src/components/LoadMore/LoadMorePaginator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const UnMemoizedLoadMorePaginator = (props: PropsWithChildren<LoadMorePag

useEffect(() => {
deprecationAndReplacementWarning([[{ refreshing }, { isLoading }]], 'LoadMorePaginator');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return (
Expand Down
1 change: 1 addition & 0 deletions src/components/Message/MessageText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const UnMemoizedMessageTextComponent = <
const messageTextToRender =
message.i18n?.[`${userLanguage}_text` as `${TranslationLanguages}_text`] || message.text;

// eslint-disable-next-line react-hooks/exhaustive-deps
const messageText = useMemo(() => renderText(messageTextToRender, message.mentioned_users), [
message.mentioned_users,
messageTextToRender,
Expand Down
2 changes: 2 additions & 0 deletions src/components/Message/hooks/useReactionHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export const useReactionHandler = <
reaction_scores: newReactionCounts,
} as StreamMessage<StreamChatGenerics>;
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[client.user, client.userID],
);

Expand Down Expand Up @@ -163,6 +164,7 @@ export const useReactionClick = <

setShowDetailedReactions(false);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[setShowDetailedReactions, reactionSelectorRef],
);

Expand Down
1 change: 1 addition & 0 deletions src/components/MessageInput/MessageInputFlat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const MessageInputFlat = <
channel?.off('message.deleted', handleQuotedMessageUpdate);
channel?.off('message.updated', handleQuotedMessageUpdate);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [channel, quotedMessage]);

return themeVersion === '2' ? (
Expand Down
1 change: 1 addition & 0 deletions src/components/MessageInput/MessageInputSmall.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const MessageInputSmall = <
channel?.off('message.deleted', handleQuotedMessageUpdate);
channel?.off('message.updated', handleQuotedMessageUpdate);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [channel, quotedMessage]);
return (
<div className='str-chat__small-message-input__wrapper'>
Expand Down
Loading

0 comments on commit d07861f

Please sign in to comment.