Skip to content

Commit

Permalink
Simplify list logic further to prevent misuse (#3334)
Browse files Browse the repository at this point in the history
* simplify list logic further

more simplification

simplify by removing `isEmpty`

use `isFetchingNextPage` everywhere for clarity

change `isFetching` to `isFetchingNextPage` for clarity

remove some useless `useMemo`s

move `renderItem` and `keyExtractor` out of component

* clean bundle size check

* update deploy

* adjust

* adjust

* one test

* try now

* test it

* done
  • Loading branch information
haileyok authored Apr 4, 2024
1 parent b1bd7ab commit 8e393b1
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 255 deletions.
19 changes: 10 additions & 9 deletions .github/workflows/bundle-deploy-eas-update.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,53 +82,54 @@ jobs:
uses: expo/expo-github-action/fingerprint@main
with:
previous-git-commit: ${{ steps.base-commit.outputs.base-commit }}
args:

- name: 👀 Debug fingerprint
id: fingerprint-debug
run: |
echo "fingerprint-diff=${{ steps.fingerprint.outputs.fingerprint-diff }}"
echo "previousGitCommit=${{ steps.fingerprint.outputs.previous-git-commit }} currentGitCommit=${{ steps.fingerprint.outputs.current-git-commit }}"
echo "isPreviousFingerprintEmpty=${{ steps.fingerprint.outputs.previous-fingerprint == '' }}"
if [ "${{ steps.fingerprint.outputs.fingerprint-diff }}" != '[]' ]; then
fingerprintDiff="${{ steps.fingerprint.outputs.fingerprint-diff }}"
if [[ $fingerprintDiff =~ "bareRncliAutolinking" || $fingerprintDiff =~ "expoAutolinkingAndroid" || $fingerprintDiff =~ "expoAutolinkingIos" ]]; then
echo fingerprint-is-different="true" >> "$GITHUB_OUTPUT"
else
echo fingerprint-is-different="false" >> "$GITHUB_OUTPUT"
fi
- name: 🔨 Setup EAS
uses: expo/expo-github-action@v8
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
with:
expo-version: latest
eas-version: latest
token: ${{ secrets.EXPO_TOKEN }}

- name: ⛏️ Setup Expo
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
run: yarn global add eas-cli-local-build-plugin

- name: 🪛 Setup jq
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
uses: dcarbone/install-jq-action@v2

- name: 🔤 Compile Translations
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
run: yarn intl:build

- name: ✏️ Write environment variables
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
run: |
export json='${{ secrets.GOOGLE_SERVICES_TOKEN }}'
echo "${{ secrets.ENV_TOKEN }}" > .env
echo "$json" > google-services.json
- name: 🏗️ Create Bundle
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
run: EXPO_PUBLIC_ENV="${{ inputs.channel || 'testflight' }}" yarn export

- name: 📦 Package Bundle and 🚀 Deploy
if: ${{ steps.fingerprint.outputs.fingerprint-diff == '[]' }}
if: ${{ steps.fingerprint-debug.outputs.fingerprint-is-different == 'false'}}
run: yarn use-build-number bash scripts/bundleUpdate.sh
env:
DENIS_API_KEY: ${{ secrets.DENIS_API_KEY }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pull-request-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
with:
base_path: 'stats-base.json'
pr_path: '../stats-new.json'
excluded_assets: '(.+).js.map|(.+).json|(.+).png'
excluded_assets: '(.+).chunk.js|(.+).js.map|(.+).json|(.+).png'

- name: 🔍 Find old comment if it exists
uses: peter-evans/find-comment@v2
Expand All @@ -99,7 +99,7 @@ jobs:
const body = `<!-- webpack-analyzer comment -->
| Old size | New size | Diff |
|----------|----------|-----------------------|
| ${{ steps.get-diff.outputs.base_file_string }} | ${{ steps.get-diff.outputs.pr_file_string }} | ${{ steps.get-diff.outputs.diff_file_string }} (${{ steps.get-diff.outputs.percent }}% |
| ${{ steps.get-diff.outputs.base_file_string }} | ${{ steps.get-diff.outputs.pr_file_string }} | ${{ steps.get-diff.outputs.diff_file_string }} (${{ steps.get-diff.outputs.percent }}%) |
`;
github.rest.issues.createComment({
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"perf:test:results": "NODE_ENV=test flashlight report .perf/results.json",
"perf:measure": "NODE_ENV=test flashlight measure",
"intl:build": "yarn intl:extract && yarn intl:compile",
"intl:check": "yarn intl:extract && git diff-index -G'(^[^\\*# /])|(^#\\w)|(^\\s+[^\\*#/])' HEAD || (echo '\n⚠️ i18n detected un-extracted translations\n' && exit 1)",
"intl:extract": "lingui extract",
"intl:compile": "lingui compile",
"nuke": "rm -rf ./node_modules && rm -rf ./ios && rm -rf ./android",
Expand Down
4 changes: 2 additions & 2 deletions src/components/Error.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React from 'react'
import {View} from 'react-native'
import {useNavigation} from '@react-navigation/core'
import {StackActions} from '@react-navigation/native'
import {msg, Trans} from '@lingui/macro'
import {useLingui} from '@lingui/react'
import {useNavigation} from '@react-navigation/core'
import {StackActions} from '@react-navigation/native'

import {NavigationProp} from 'lib/routes/types'
import {CenteredView} from 'view/com/util/Views'
Expand Down
96 changes: 47 additions & 49 deletions src/components/LikedByList.tsx
Original file line number Diff line number Diff line change
@@ -1,47 +1,54 @@
import React from 'react'
import {View} from 'react-native'
import {AppBskyFeedGetLikes as GetLikes} from '@atproto/api'
import {Trans} from '@lingui/macro'
import {msg} from '@lingui/macro'
import {useLingui} from '@lingui/react'

import {logger} from '#/logger'
import {List} from '#/view/com/util/List'
import {ProfileCardWithFollowBtn} from '#/view/com/profile/ProfileCard'
import {useResolveUriQuery} from '#/state/queries/resolve-uri'
import {useLikedByQuery} from '#/state/queries/post-liked-by'
import {useResolveUriQuery} from '#/state/queries/resolve-uri'
import {useInitialNumToRender} from 'lib/hooks/useInitialNumToRender'
import {ListFooter} from '#/components/Lists'
import {cleanError} from 'lib/strings/errors'
import {ProfileCardWithFollowBtn} from '#/view/com/profile/ProfileCard'
import {List} from '#/view/com/util/List'
import {ListFooter, ListMaybePlaceholder} from '#/components/Lists'

import {atoms as a, useTheme} from '#/alf'
import {Loader} from '#/components/Loader'
import {Text} from '#/components/Typography'
function renderItem({item}: {item: GetLikes.Like}) {
return <ProfileCardWithFollowBtn key={item.actor.did} profile={item.actor} />
}

function keyExtractor(item: GetLikes.Like) {
return item.actor.did
}

export function LikedByList({uri}: {uri: string}) {
const t = useTheme()
const {_} = useLingui()
const initialNumToRender = useInitialNumToRender()
const [isPTRing, setIsPTRing] = React.useState(false)

const {
data: resolvedUri,
error: resolveError,
isFetching: isFetchingResolvedUri,
isLoading: isUriLoading,
} = useResolveUriQuery(uri)
const {
data,
isFetching,
isFetched,
isRefetching,
isLoading: isLikedByLoading,
isFetchingNextPage,
hasNextPage,
fetchNextPage,
isError,
error: likedByError,
refetch,
} = useLikedByQuery(resolvedUri?.uri)

const error = resolveError || likedByError
const isError = !!resolveError || !!likedByError

const likes = React.useMemo(() => {
if (data?.pages) {
return data.pages.flatMap(page => page.likes)
}
return []
}, [data])
const initialNumToRender = useInitialNumToRender()
const error = resolveError || likedByError

const onRefresh = React.useCallback(async () => {
setIsPTRing(true)
Expand All @@ -54,56 +61,47 @@ export function LikedByList({uri}: {uri: string}) {
}, [refetch, setIsPTRing])

const onEndReached = React.useCallback(async () => {
if (isFetching || !hasNextPage || isError) return
if (isFetchingNextPage || !hasNextPage || isError) return
try {
await fetchNextPage()
} catch (err) {
logger.error('Failed to load more likes', {message: err})
}
}, [isFetching, hasNextPage, isError, fetchNextPage])

const renderItem = React.useCallback(({item}: {item: GetLikes.Like}) => {
return (
<ProfileCardWithFollowBtn key={item.actor.did} profile={item.actor} />
)
}, [])
}, [isFetchingNextPage, hasNextPage, isError, fetchNextPage])

if (isFetchingResolvedUri || !isFetched) {
if (likes.length < 1) {
return (
<View style={[a.w_full, a.align_center, a.p_lg]}>
<Loader size="xl" />
</View>
<ListMaybePlaceholder
isLoading={isUriLoading || isLikedByLoading}
isError={isError}
emptyType="results"
emptyMessage={_(
msg`Nobody has liked this yet. Maybe you should be the first!`,
)}
errorMessage={cleanError(resolveError || error)}
onRetry={isError ? refetch : undefined}
/>
)
}

return likes.length ? (
return (
<List
data={likes}
keyExtractor={item => item.actor.did}
renderItem={renderItem}
keyExtractor={keyExtractor}
refreshing={isPTRing}
onRefresh={onRefresh}
onEndReached={onEndReached}
onEndReachedThreshold={3}
renderItem={renderItem}
initialNumToRender={initialNumToRender}
ListFooterComponent={() => (
ListFooterComponent={
<ListFooter
isFetching={isFetching && !isRefetching}
isError={isError}
error={error ? error.toString() : undefined}
isFetchingNextPage={isFetchingNextPage}
error={cleanError(error)}
onRetry={fetchNextPage}
/>
)}
}
onEndReachedThreshold={3}
initialNumToRender={initialNumToRender}
windowSize={11}
/>
) : (
<View style={[a.p_lg]}>
<View style={[a.p_lg, a.rounded_sm, t.atoms.bg_contrast_25]}>
<Text style={[a.text_md, a.leading_snug]}>
<Trans>
Nobody has liked this yet. Maybe you should be the first!
</Trans>
</Text>
</View>
</View>
)
}
56 changes: 25 additions & 31 deletions src/components/Lists.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
import React from 'react'
import {atoms as a, useBreakpoints, useTheme} from '#/alf'
import {View} from 'react-native'
import {msg, Trans} from '@lingui/macro'
import {useLingui} from '@lingui/react'
import {Trans, msg} from '@lingui/macro'

import {CenteredView} from 'view/com/util/Views'
import {Loader} from '#/components/Loader'
import {cleanError} from 'lib/strings/errors'
import {CenteredView} from 'view/com/util/Views'
import {atoms as a, useBreakpoints, useTheme} from '#/alf'
import {Button} from '#/components/Button'
import {Text} from '#/components/Typography'
import {Error} from '#/components/Error'
import {Loader} from '#/components/Loader'
import {Text} from '#/components/Typography'

export function ListFooter({
isFetching,
isError,
isFetchingNextPage,
error,
onRetry,
height,
}: {
isFetching?: boolean
isError?: boolean
isFetchingNextPage?: boolean
error?: string
onRetry?: () => Promise<unknown>
height?: number
Expand All @@ -36,32 +34,26 @@ export function ListFooter({
t.atoms.border_contrast_low,
{height: height ?? 180, paddingTop: 30},
]}>
{isFetching ? (
{isFetchingNextPage ? (
<Loader size="xl" />
) : (
<ListFooterMaybeError
isError={isError}
error={error}
onRetry={onRetry}
/>
<ListFooterMaybeError error={error} onRetry={onRetry} />
)}
</View>
)
}

function ListFooterMaybeError({
isError,
error,
onRetry,
}: {
isError?: boolean
error?: string
onRetry?: () => Promise<unknown>
}) {
const t = useTheme()
const {_} = useLingui()

if (!isError) return null
if (!error) return null

return (
<View style={[a.w_full, a.px_lg]}>
Expand Down Expand Up @@ -128,7 +120,7 @@ export function ListHeaderDesktop({

export function ListMaybePlaceholder({
isLoading,
isEmpty,
noEmpty,
isError,
emptyTitle,
emptyMessage,
Expand All @@ -138,7 +130,7 @@ export function ListMaybePlaceholder({
onRetry,
}: {
isLoading: boolean
isEmpty?: boolean
noEmpty?: boolean
isError?: boolean
emptyTitle?: string
emptyMessage?: string
Expand All @@ -151,16 +143,6 @@ export function ListMaybePlaceholder({
const {_} = useLingui()
const {gtMobile, gtTablet} = useBreakpoints()

if (!isLoading && isError) {
return (
<Error
title={errorTitle ?? _(msg`Oops!`)}
message={errorMessage ?? _(`Something went wrong!`)}
onRetry={onRetry}
/>
)
}

if (isLoading) {
return (
<CenteredView
Expand All @@ -180,7 +162,17 @@ export function ListMaybePlaceholder({
)
}

if (isEmpty) {
if (isError) {
return (
<Error
title={errorTitle ?? _(msg`Oops!`)}
message={errorMessage ?? _(`Something went wrong!`)}
onRetry={onRetry}
/>
)
}

if (!noEmpty) {
return (
<Error
title={
Expand All @@ -197,4 +189,6 @@ export function ListMaybePlaceholder({
/>
)
}

return null
}
2 changes: 1 addition & 1 deletion src/lib/icons.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react'
import {StyleProp, TextStyle, ViewStyle} from 'react-native'
import Svg, {Path, Rect, Line, Ellipse} from 'react-native-svg'
import Svg, {Ellipse, Line, Path, Rect} from 'react-native-svg'

export function GridIcon({
style,
Expand Down
Loading

0 comments on commit 8e393b1

Please sign in to comment.