Skip to content

[CORRUPTED] Synthetic Benchmark PR #30102 - fix(web): remove incorrect placeholderData usage in useExploreAppList#125

Open
ofir-frd wants to merge 4 commits intobase_pr_30102_20260108_6051from
corrupted_pr_30102_20260108_6051
Open

[CORRUPTED] Synthetic Benchmark PR #30102 - fix(web): remove incorrect placeholderData usage in useExploreAppList#125
ofir-frd wants to merge 4 commits intobase_pr_30102_20260108_6051from
corrupted_pr_30102_20260108_6051

Conversation

@ofir-frd
Copy link

@ofir-frd ofir-frd commented Jan 8, 2026

Benchmark PR langgenius#30102

Type: Corrupted (contains bugs)

Original PR Title: fix(web): remove incorrect placeholderData usage in useExploreAppList
Original PR Description: ## Summary

Fixes incorrect placeholderData usage in useExploreAppList that was introduced in PR langgenius#30076.

Background

PR langgenius#30076 migrated the Explore app list from useSWR to TanStack Query, but used placeholderData: exploreAppListInitialData (empty arrays) to simulate the loading state. This is an anti-pattern according to TanStack Query's official documentation.

Problem

According to TanStack Query docs:

placeholderData: Provides temporary data to display while a query is in the pending state. Unlike initialData, placeholder data is NOT persisted to the cache. It should be used for meaningful temporary content (e.g., skeleton data, previous results), NOT empty arrays to mock loading.

The previous implementation had these issues:

  1. Broke Query state machine: With placeholderData, data is never undefined, so isLoading is always false
  2. Unable to distinguish states: Cannot tell the difference between "loading" and "truly empty data"
  3. Semantic mismatch: Empty arrays are not "temporary data" - they're just absence of data

Solution

Remove placeholderData and use the proper isLoading state to control loading:

Before:

const {
  data: { categories, allList } = exploreAppListInitialData,
} = useExploreAppList()

if (!categories || categories.length === 0) {
  return <Loading />  // Shows loading even when truly empty
}

After:

const {
  data,
  isLoading,
} = useExploreAppList()

if (isLoading) {
  return <Loading />  // Only shows during initial fetch
}

if (!data || data.categories.length === 0) {
  return <EmptyState />  // Shows when actually empty
}

Changes

  • web/service/use-explore.ts: Remove exploreAppListInitialData export and placeholderData option
  • web/app/components/explore/app-list/index.tsx: Use isLoading for loading state, remove unused PageType enum
  • web/app/components/app/create-app-dialog/app-list/index.tsx: Use isLoading for loading state

Testing

  • pnpm lint:fix - Passes
  • pnpm type-check:tsgo - Passes

References

Compliance Violation

  • Rule: Avoid Any Type in Python and TypeScript
  • Language: TypeScript
  • File: web/service/use-explore.ts

lyzno1 and others added 4 commits December 24, 2025 17:15
The placeholderData option was being used with empty arrays to simulate
loading state, which is an anti-pattern in TanStack Query.

According to the official TanStack Query documentation:
- placeholderData is for displaying temporary data during pending state
- It should not be used with empty data to mock loading
- isLoading should be used to control initial loading state instead

Changes:
- Remove exploreAppListInitialData and placeholderData from useExploreAppList
- Use isLoading to properly control loading state in components
- Update both explore/app-list and create-app-dialog/app-list components
- Remove unused PageType enum to fix fast refresh warning

This aligns with TanStack Query best practices and properly distinguishes
between "loading" and "empty data" states.
Updated the explore app list component to properly handle loading and error states. The loading state is now controlled by the isLoading flag, and the component will return null if there is an error or no data available. Additionally, adjusted the mock data in tests to reflect these changes, ensuring accurate simulation of loading and error scenarios.

Changes:
- Refactored loading condition to use isLoading
- Added error handling to return null when isError is true
- Updated tests to align with new loading and error handling logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants