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

Simplify fetchState in the Nuxt app #5322

Open
obulat opened this issue Jan 8, 2025 · 0 comments · May be fixed by #5323
Open

Simplify fetchState in the Nuxt app #5322

obulat opened this issue Jan 8, 2025 · 0 comments · May be fixed by #5323
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend

Comments

@obulat
Copy link
Contributor

obulat commented Jan 8, 2025

Problem

The current structure of fetchState uses multiple flags (e.g., isFetching, hasStarted, isFinished, fetchingError) to manage the state of API requests. This approach:

  • Increases complexity and redundancy.
  • Makes it harder to reason about the current state of a request.
  • Lacks explicit type safety, leading to potential inconsistencies (e.g., isFinished: true with a non-null fetchingError).

Description

Refactor all fetchState objects to use a discriminated union, which explicitly defines possible states and ensures type safety. The new structure will look like this:

export type FetchState =
  | { status: 'idle' | 'loading' | 'success'; error: null }
  | { status: 'error'; error: FetchingError };

Benefits

  • Simplified state management: The status field alone determines the fetch state, eliminating the need for redundant flags.
  • Explicit error handling: The error field is only available when status is 'error', reducing ambiguity and improving clarity.
  • Type safety: Ensures that invalid combinations (e.g., status: 'success' with error) are impossible.

This change will apply to all instances of fetchState objects across the codebase.

Alternatives

Keep the Current Structure

  • Pros:
    • No immediate changes required.
  • Cons:
    • Complexity and redundancy remain.
    • Potential for invalid state combinations persists.

Additional Context

Derived states (getters) for the UI

This change will also simplify the following common derived states:

canLoadMore

Determines whether to display the "Load More" button:

const canLoadMore = (currentPage: number, totalPages: number) => {
  return currentPage < totalPages;
};

showLoadingSkeleton

Loading skeleton is only displayed for the first page being loaded. This getter will be used to determine whether to display it:

const showLoadingSkeleton = (fetchState: FetchState, currentPage: number) => {
  return fetchState.status === 'loading' && currentPage === 0;
};

showNoResultsPage

Determines if the “No Results” page should be displayed:

const showNoResultsPage = (fetchState: FetchState, totalResults: number) => {
  return fetchState.status === 'success' && totalResults === 0;
};

Derived state for the store

To determine whether to send a request for the next page for specific media type, the following getter can be used:

const canFetch = (type: MediaType) => {
  const fetchState = this.mediaFetchState[type];

  return (
    fetchState.status !== 'error' && // No unretriable error
    fetchState.status !== 'loading' && // Not already fetching
    !(this.results[type].page >= this.results[type].pageCount) // Not finished
  );
};
@obulat obulat added 💻 aspect: code Concerns the software code in the repository 🟨 priority: medium Not blocking but should be addressed soon 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: frontend Related to the Nuxt frontend labels Jan 8, 2025
@obulat obulat self-assigned this Jan 8, 2025
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Jan 8, 2025
@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Jan 8, 2025
@obulat obulat linked a pull request Jan 8, 2025 that will close this issue
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Status: 🏗 In Progress
Development

Successfully merging a pull request may close this issue.

1 participant