Skip to content

add fallback behaviour for is-loading #20

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 21, 2025
Merged

add fallback behaviour for is-loading #20

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c31023
make is-loading optional
lukasvice Nov 21, 2025
9ef3227
make data, error and is-loading required but undefindable
lukasvice Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/async-data/AsyncView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
import React, { ReactElement, ReactNode } from 'react'
import { ReactElement, ReactNode } from 'react'
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The React import was removed but ReactElement and ReactNode types are still being imported. Since this component returns JSX (line 43, 47), React needs to be in scope for older React versions or the JSX transform configuration should be verified to ensure the automatic JSX runtime is enabled.

Suggested change
import { ReactElement, ReactNode } from 'react'
import React, { ReactElement, ReactNode } from 'react'

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@lukasvice lukasvice Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ok, I guess. It was automatically removed.

import { isFunction } from '../util'

type LoadingFunction = () => ReactNode
type SuccessFunction<Data> = (data: Data) => ReactNode
type ErrorFunction<Error> = (error: NonNullable<Error>) => ReactNode

type Props<Data, Error> = {
data?: Data
error?: Error
isLoading: boolean
/**
* The async data to show.
* Note that `error` and `isLoading` take precedence over data.
*/
data: Data | undefined
/**
* Set an error to show the error state.
* Note that `isLoading` takes precedence over error.
*/
error: Error | undefined
/**
* Set to a boolean value to explicitly control loading state.
* If undefined, loading state is shown when there is no data (null | undefined) and no error (null | undefined).
*/
isLoading: boolean | undefined
renderLoading?: ReactNode | LoadingFunction
renderError?: ReactNode | ErrorFunction<Error>
} & (
Expand Down Expand Up @@ -36,15 +48,18 @@ const AsyncView = <Data, Error>(
allowMissingData = false,
} = props

if (isLoading) {
const isError = error !== null && error !== undefined
const hasData = data !== null && data !== undefined

if (isLoading || (isLoading === undefined && !hasData && !isError)) {
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback loading state logic !hasData && !isError could incorrectly treat the initial state (no data, no error) as loading even after a request completes with empty/falsy data. Consider whether empty arrays, empty strings, or false as valid data values should trigger loading state.

Suggested change
if (isLoading || (isLoading === undefined && !hasData && !isError)) {
// Show loading only if isLoading is true, or if isLoading is undefined and both data and error are undefined/null (initial state)
if (isLoading === true || (isLoading === undefined && !hasData && !isError)) {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@lukasvice lukasvice Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true, as we check explicitly for null and undefined.

return <>{isFunction(renderLoading) ? renderLoading() : renderLoading}</>
}

if (error !== null && error !== undefined) {
if (isError) {
return <>{isFunction(renderError) ? renderError(error) : renderError}</>
}

if ((data === undefined || data === null) && !allowMissingData) {
if (!hasData && !allowMissingData) {
throw new Error(
'Data passed into AsyncView was null or undefined. Use allowMissingData=true if this is intended.',
)
Expand Down