From b2530dadd512f2191bce49766b2c2c3cc3bd4b2c Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 09:35:07 -0400 Subject: [PATCH 1/8] Convert trends to React Query --- .../features/ui/components/trends-panel.tsx | 18 +++----- app/soapbox/queries/__tests__/trends.test.ts | 42 +++++++++++++++++++ app/soapbox/queries/trends.ts | 28 +++++++++++++ 3 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 app/soapbox/queries/__tests__/trends.test.ts create mode 100644 app/soapbox/queries/trends.ts diff --git a/app/soapbox/features/ui/components/trends-panel.tsx b/app/soapbox/features/ui/components/trends-panel.tsx index 49b68887d..ee0d8d763 100644 --- a/app/soapbox/features/ui/components/trends-panel.tsx +++ b/app/soapbox/features/ui/components/trends-panel.tsx @@ -1,40 +1,32 @@ import * as React from 'react'; import { FormattedMessage } from 'react-intl'; -import { useDispatch } from 'react-redux'; -import { fetchTrends } from 'soapbox/actions/trends'; import Hashtag from 'soapbox/components/hashtag'; import { Widget } from 'soapbox/components/ui'; -import { useAppSelector } from 'soapbox/hooks'; +import useTrends from 'soapbox/queries/trends'; interface ITrendsPanel { limit: number } const TrendsPanel = ({ limit }: ITrendsPanel) => { - const dispatch = useDispatch(); - - const trends = useAppSelector((state) => state.trends.items); + const { data: trends, isFetching } = useTrends(); const sortedTrends = React.useMemo(() => { - return trends.sort((a, b) => { + return trends?.sort((a, b) => { const num_a = Number(a.getIn(['history', 0, 'accounts'])); const num_b = Number(b.getIn(['history', 0, 'accounts'])); return num_b - num_a; }).slice(0, limit); }, [trends, limit]); - React.useEffect(() => { - dispatch(fetchTrends()); - }, []); - - if (sortedTrends.isEmpty()) { + if (trends?.length === 0 || isFetching) { return null; } return ( }> - {sortedTrends.map((hashtag) => ( + {sortedTrends?.slice(0, limit).map((hashtag) => ( ))} diff --git a/app/soapbox/queries/__tests__/trends.test.ts b/app/soapbox/queries/__tests__/trends.test.ts new file mode 100644 index 000000000..52f319d26 --- /dev/null +++ b/app/soapbox/queries/__tests__/trends.test.ts @@ -0,0 +1,42 @@ +import { __stub } from 'soapbox/api'; +import { renderHook, waitFor } from 'soapbox/jest/test-helpers'; + +import useTrends from '../trends'; + +describe('useTrends', () => { + describe('with a successul query', () => { + beforeEach(() => { + __stub((mock) => { + mock.onGet('/api/v1/trends') + .reply(200, [ + { name: '#golf', url: 'https://example.com' }, + { name: '#tennis', url: 'https://example.com' }, + ]); + }); + }); + + it('is successful', async() => { + const { result } = renderHook(() => useTrends()); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(result.current.data?.length).toBe(2); + }); + }); + + describe('with an unsuccessul query', () => { + beforeEach(() => { + __stub((mock) => { + mock.onGet('/api/v1/trends').networkError(); + }); + }); + + it('is successful', async() => { + const { result } = renderHook(() => useTrends()); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(result.current.error).toBeDefined(); + }); + }); +}); diff --git a/app/soapbox/queries/trends.ts b/app/soapbox/queries/trends.ts new file mode 100644 index 000000000..8613719b0 --- /dev/null +++ b/app/soapbox/queries/trends.ts @@ -0,0 +1,28 @@ +import { useQuery } from '@tanstack/react-query'; + +import { fetchTrendsSuccess } from 'soapbox/actions/trends'; +import { useApi, useAppDispatch } from 'soapbox/hooks'; +import { normalizeTag } from 'soapbox/normalizers'; + +import type { Tag } from 'soapbox/types/entities'; + +export default function useTrends() { + const api = useApi(); + const dispatch = useAppDispatch(); + + const getTrends = async() => { + const { data } = await api.get('/api/v1/trends'); + + dispatch(fetchTrendsSuccess(data)); + + const normalizedData = data.map((tag) => normalizeTag(tag)); + return normalizedData; + }; + + const result = useQuery(['trends'], getTrends, { + placeholderData: [], + staleTime: 600000, // 10 minutes + }); + + return result; +} \ No newline at end of file From 968ec3a7d2255444f8a38b1f84e92c67a8ef0bd8 Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 10:30:58 -0400 Subject: [PATCH 2/8] Clear React Query cache before each test --- app/soapbox/jest/test-helpers.tsx | 3 +++ app/soapbox/jest/test-setup.ts | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/soapbox/jest/test-helpers.tsx b/app/soapbox/jest/test-helpers.tsx index 0894d4d40..721783879 100644 --- a/app/soapbox/jest/test-helpers.tsx +++ b/app/soapbox/jest/test-helpers.tsx @@ -37,6 +37,8 @@ const queryClient = new QueryClient({ }, defaultOptions: { queries: { + staleTime: 0, + cacheTime: Infinity, retry: false, }, }, @@ -123,4 +125,5 @@ export { rootReducer, mockWindowProperty, createTestStore, + queryClient, }; diff --git a/app/soapbox/jest/test-setup.ts b/app/soapbox/jest/test-setup.ts index 0052388b0..cf24ba3f1 100644 --- a/app/soapbox/jest/test-setup.ts +++ b/app/soapbox/jest/test-setup.ts @@ -2,9 +2,14 @@ import { __clear as clearApiMocks } from '../__mocks__/api'; +import { queryClient } from './test-helpers'; + // API mocking jest.mock('soapbox/api'); -afterEach(() => clearApiMocks()); +afterEach(() => { + clearApiMocks(); + queryClient.clear(); +}); // Mock IndexedDB // https://dev.to/andyhaskell/testing-your-indexeddb-code-with-jest-2o17 From cbe9f47a59270b10b6b31f30cef4f26cd566028c Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 10:31:09 -0400 Subject: [PATCH 3/8] Fix Trends Panel test --- .../__tests__/trends-panel.test.tsx | 131 ++++++++---------- 1 file changed, 58 insertions(+), 73 deletions(-) diff --git a/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx b/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx index 487a84bfb..20eb3c5af 100644 --- a/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx +++ b/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx @@ -1,85 +1,70 @@ -import { List as ImmutableList, Record as ImmutableRecord } from 'immutable'; import React from 'react'; -import { render, screen } from '../../../../jest/test-helpers'; -import { normalizeTag } from '../../../../normalizers'; +import { __stub } from 'soapbox/api'; + +import { render, screen, waitFor } from '../../../../jest/test-helpers'; import TrendsPanel from '../trends-panel'; describe('', () => { - it('renders trending hashtags', () => { - const store = { - trends: ImmutableRecord({ - items: ImmutableList([ - normalizeTag({ - name: 'hashtag 1', - history: [{ - day: '1652745600', - uses: '294', - accounts: '180', - }], - }), - ]), - isLoading: false, - })(), - }; + describe('with hashtags', () => { + beforeEach(() => { + __stub((mock) => { + mock.onGet('/api/v1/trends') + .reply(200, [ + { + name: 'hashtag 1', + url: 'https://example.com', + history: [{ + day: '1652745600', + uses: '294', + accounts: '180', + }], + }, + { name: 'hashtag 2', url: 'https://example.com' }, + ]); + }); + }); - render(, undefined, store); - expect(screen.getByTestId('hashtag')).toHaveTextContent(/hashtag 1/i); - expect(screen.getByTestId('hashtag')).toHaveTextContent(/180 people talking/i); - expect(screen.getByTestId('sparklines')).toBeInTheDocument(); + it('renders trending hashtags', async() => { + render(); + + await waitFor(() => { + expect(screen.getByTestId('hashtag')).toHaveTextContent(/hashtag 1/i); + expect(screen.getByTestId('hashtag')).toHaveTextContent(/180 people talking/i); + expect(screen.getByTestId('sparklines')).toBeInTheDocument(); + }); + }); + + it('renders multiple trends', async() => { + render(); + + await waitFor(() => { + expect(screen.queryAllByTestId('hashtag')).toHaveLength(2); + }); + }); + + it('respects the limit prop', async() => { + render(); + + await waitFor(() => { + expect(screen.queryAllByTestId('hashtag')).toHaveLength(1); + }); + }); }); - it('renders multiple trends', () => { - const store = { - trends: ImmutableRecord({ - items: ImmutableList([ - normalizeTag({ - name: 'hashtag 1', - history: ImmutableList([{ accounts: [] }]), - }), - normalizeTag({ - name: 'hashtag 2', - history: ImmutableList([{ accounts: [] }]), - }), - ]), - isLoading: false, - })(), - }; + describe('without hashtags', () => { + beforeEach(() => { + __stub((mock) => { + mock.onGet('/api/v1/trends').reply(200, []); + }); + }); - render(, undefined, store); - expect(screen.queryAllByTestId('hashtag')).toHaveLength(2); - }); + it('renders empty', async() => { + render(); - it('respects the limit prop', () => { - const store = { - trends: ImmutableRecord({ - items: ImmutableList([ - normalizeTag({ - name: 'hashtag 1', - history: [{ accounts: [] }], - }), - normalizeTag({ - name: 'hashtag 2', - history: [{ accounts: [] }], - }), - ]), - isLoading: false, - })(), - }; - - render(, undefined, store); - expect(screen.queryAllByTestId('hashtag')).toHaveLength(1); - }); - - it('renders empty', () => { - const store = { - trends: ImmutableRecord({ - items: ImmutableList([]), - isLoading: false, - })(), - }; - - render(, undefined, store); - expect(screen.queryAllByTestId('hashtag')).toHaveLength(0); + await waitFor(() => { + expect(screen.queryAllByTestId('hashtag')).toHaveLength(0); + }); + }); }); }); From b377689ed26330e97cb650f740170643eaaf2601 Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 10:34:12 -0400 Subject: [PATCH 4/8] Grammar fix --- app/soapbox/queries/__tests__/suggestions.test.ts | 4 ++-- app/soapbox/queries/__tests__/trends.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/soapbox/queries/__tests__/suggestions.test.ts b/app/soapbox/queries/__tests__/suggestions.test.ts index 15977dbb9..f38bf0dbc 100644 --- a/app/soapbox/queries/__tests__/suggestions.test.ts +++ b/app/soapbox/queries/__tests__/suggestions.test.ts @@ -4,7 +4,7 @@ import { renderHook, waitFor } from 'soapbox/jest/test-helpers'; import useOnboardingSuggestions from '../suggestions'; describe('useCarouselAvatars', () => { - describe('with a successul query', () => { + describe('with a successful query', () => { beforeEach(() => { __stub((mock) => { mock.onGet('/api/v2/suggestions') @@ -26,7 +26,7 @@ describe('useCarouselAvatars', () => { }); }); - describe('with an unsuccessul query', () => { + describe('with an unsuccessful query', () => { beforeEach(() => { __stub((mock) => { mock.onGet('/api/v2/suggestions').networkError(); diff --git a/app/soapbox/queries/__tests__/trends.test.ts b/app/soapbox/queries/__tests__/trends.test.ts index 52f319d26..07856cedf 100644 --- a/app/soapbox/queries/__tests__/trends.test.ts +++ b/app/soapbox/queries/__tests__/trends.test.ts @@ -4,7 +4,7 @@ import { renderHook, waitFor } from 'soapbox/jest/test-helpers'; import useTrends from '../trends'; describe('useTrends', () => { - describe('with a successul query', () => { + describe('with a successful query', () => { beforeEach(() => { __stub((mock) => { mock.onGet('/api/v1/trends') @@ -24,7 +24,7 @@ describe('useTrends', () => { }); }); - describe('with an unsuccessul query', () => { + describe('with an unsuccessful query', () => { beforeEach(() => { __stub((mock) => { mock.onGet('/api/v1/trends').networkError(); From 06d1ad2efed8376448880927d2e69fca03cfeebf Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 11:33:23 -0400 Subject: [PATCH 5/8] Remove sort --- app/soapbox/features/ui/components/trends-panel.tsx | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/soapbox/features/ui/components/trends-panel.tsx b/app/soapbox/features/ui/components/trends-panel.tsx index ee0d8d763..9f582d891 100644 --- a/app/soapbox/features/ui/components/trends-panel.tsx +++ b/app/soapbox/features/ui/components/trends-panel.tsx @@ -12,21 +12,13 @@ interface ITrendsPanel { const TrendsPanel = ({ limit }: ITrendsPanel) => { const { data: trends, isFetching } = useTrends(); - const sortedTrends = React.useMemo(() => { - return trends?.sort((a, b) => { - const num_a = Number(a.getIn(['history', 0, 'accounts'])); - const num_b = Number(b.getIn(['history', 0, 'accounts'])); - return num_b - num_a; - }).slice(0, limit); - }, [trends, limit]); - if (trends?.length === 0 || isFetching) { return null; } return ( }> - {sortedTrends?.slice(0, limit).map((hashtag) => ( + {trends?.slice(0, limit).map((hashtag) => ( ))} From 4d98046627c451d74212f596dca81b79fd2a2ba1 Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 13:27:09 -0400 Subject: [PATCH 6/8] Improve types --- app/soapbox/queries/trends.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/soapbox/queries/trends.ts b/app/soapbox/queries/trends.ts index 8613719b0..afe780991 100644 --- a/app/soapbox/queries/trends.ts +++ b/app/soapbox/queries/trends.ts @@ -11,7 +11,7 @@ export default function useTrends() { const dispatch = useAppDispatch(); const getTrends = async() => { - const { data } = await api.get('/api/v1/trends'); + const { data } = await api.get('/api/v1/trends'); dispatch(fetchTrendsSuccess(data)); @@ -19,10 +19,10 @@ export default function useTrends() { return normalizedData; }; - const result = useQuery(['trends'], getTrends, { + const result = useQuery>(['trends'], getTrends, { placeholderData: [], staleTime: 600000, // 10 minutes }); return result; -} \ No newline at end of file +} From 6c03b6ddd34d39116585d5ae7eabe1cbd86458ff Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 14:02:29 -0400 Subject: [PATCH 7/8] fix tests --- app/soapbox/jest/test-setup.ts | 7 +------ app/soapbox/queries/__tests__/trends.test.ts | 6 +++++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/soapbox/jest/test-setup.ts b/app/soapbox/jest/test-setup.ts index cf24ba3f1..0052388b0 100644 --- a/app/soapbox/jest/test-setup.ts +++ b/app/soapbox/jest/test-setup.ts @@ -2,14 +2,9 @@ import { __clear as clearApiMocks } from '../__mocks__/api'; -import { queryClient } from './test-helpers'; - // API mocking jest.mock('soapbox/api'); -afterEach(() => { - clearApiMocks(); - queryClient.clear(); -}); +afterEach(() => clearApiMocks()); // Mock IndexedDB // https://dev.to/andyhaskell/testing-your-indexeddb-code-with-jest-2o17 diff --git a/app/soapbox/queries/__tests__/trends.test.ts b/app/soapbox/queries/__tests__/trends.test.ts index 07856cedf..784f928cb 100644 --- a/app/soapbox/queries/__tests__/trends.test.ts +++ b/app/soapbox/queries/__tests__/trends.test.ts @@ -1,9 +1,13 @@ import { __stub } from 'soapbox/api'; -import { renderHook, waitFor } from 'soapbox/jest/test-helpers'; +import { queryClient, renderHook, waitFor } from 'soapbox/jest/test-helpers'; import useTrends from '../trends'; describe('useTrends', () => { + beforeEach(() => { + queryClient.clear(); + }); + describe('with a successful query', () => { beforeEach(() => { __stub((mock) => { From 22294b8a6e33c82046fcc4601a2525d24029ff00 Mon Sep 17 00:00:00 2001 From: Justin Date: Wed, 10 Aug 2022 14:45:58 -0400 Subject: [PATCH 8/8] Fix test --- .../features/ui/components/__tests__/trends-panel.test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx b/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx index 20eb3c5af..0d203ab9b 100644 --- a/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx +++ b/app/soapbox/features/ui/components/__tests__/trends-panel.test.tsx @@ -2,10 +2,14 @@ import React from 'react'; import { __stub } from 'soapbox/api'; -import { render, screen, waitFor } from '../../../../jest/test-helpers'; +import { queryClient, render, screen, waitFor } from '../../../../jest/test-helpers'; import TrendsPanel from '../trends-panel'; describe('', () => { + beforeEach(() => { + queryClient.clear(); + }); + describe('with hashtags', () => { beforeEach(() => { __stub((mock) => {