Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const App = () => <ReactSVG src="svg.svg" />
| [`fallback`](#fallback) | `React.ElementType` | none |
| [`httpRequestWithCredentials`](#httprequestwithcredentials) | `boolean` | `false` |
| [`loading`](#loading) | `React.ElementType` | none |
| [`loadingDelay`](#loadingdelay) | `number` | `0` |
| [`onError`](#onerror) | `(error: unknown) => void` | noop |
| [`renumerateIRIElements`](#renumerateirielements) | `boolean` | `true` |
| [`title`](#title) | `string` | `''` |
Expand Down Expand Up @@ -84,6 +85,14 @@ Whether cross-site Access-Control requests for the SVG are made using credential

Rendered inside the wrapper until the SVG is injected. Can be a string, class component or function component. Nothing is rendered in its place when unset.

#### `loadingDelay`

Milliseconds to wait before rendering `loading`. At the default `0` it renders immediately. Set a value and an injection that finishes sooner - a warm request cache, a localhost or `file://` read, a warm CDN edge - never renders `loading` at all.

Use it when `loading` is a spinner, where a sub-second appearance is worse than none: the user can't tell what flashed. Leave it at `0` when `loading` is a skeleton sized to hold the SVG's space, because delaying that trades one layout shift for two.

200-300ms is the usual industry choice. The delay applies to `loading` only - `fallback` always renders as soon as the error arrives, since an error costs a round trip that no cache short-circuits.

#### `onError`

Called if an error occurs. `error` is an `unknown` value.
Expand Down
84 changes: 81 additions & 3 deletions src/ReactSVG.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const ReactSVG: React.ForwardRefExoticComponent<
fallback: Fallback,
httpRequestWithCredentials = false,
loading: Loading,
loadingDelay = 0,
onError = () => undefined,
renumerateIRIElements = true,
src,
Expand All @@ -44,6 +45,27 @@ export const ReactSVG: React.ForwardRefExoticComponent<
const [hasError, setHasError] = React.useState(false)
const [isLoading, setIsLoading] = React.useState(true)

// `loading` is held back until the delay elapses, so an injection that
// resolves sooner - a warm cache, localhost, a file:// read - never paints
// an indicator at all. The initial value has to account for the delay
// rather than start false: effects run after paint, so initialising to
// false would cost the default a frame without the loader.
const [hasLoadingDelayElapsed, setHasLoadingDelayElapsed] = React.useState(
loadingDelay <= 0,
)

// Bumped whenever an injection starts, so the delay effect below can tell
// one injection from the next. `isLoading` can't carry that on its own: a
// re-injection that begins while the previous request is still in flight
// leaves it true throughout, so with an unchanged `loadingDelay` neither of
// that effect's other dependencies would change and the timer would never
// re-arm.
//
// Skipped for the first injection, so mount still settles in one render -
// every value that effect writes already holds its default there.
const [injectionId, setInjectionId] = React.useState(0)
const hasInjectedRef = React.useRef(false)

const reactWrapperRef = React.useRef<WrapperType | null>(null)

// The callbacks are read through a ref so that changing them - which inline
Expand All @@ -59,6 +81,15 @@ export const ReactSVG: React.ForwardRefExoticComponent<
callbacksRef.current = { afterInjection, beforeInjection, onError }
})

// Read through a ref for the same reason, and declared here so it is
// current by the time the injection effect runs: that effect has to clear
// the elapsed flag, but listing `loadingDelay` as a dependency would make
// changing the delay re-inject.
const loadingDelayRef = React.useRef(loadingDelay)
React.useEffect(() => {
loadingDelayRef.current = loadingDelay
})

const refCallback = React.useCallback(
(reactWrapper: WrapperType | null) => {
reactWrapperRef.current = reactWrapper
Expand Down Expand Up @@ -97,11 +128,26 @@ export const ReactSVG: React.ForwardRefExoticComponent<
}

// A new injection is starting, so any result from the previous one is
// stale. On mount both values already hold these defaults and React bails
// out, so this only re-renders when a dependency actually changed.
// stale. On mount the three flags already hold these defaults and React
// bails out - and `injectionId` is skipped there for the same reason - so
// this only re-renders when a dependency actually changed.
//
// The elapsed flag has to be cleared here rather than left to the delay
// effect below: that effect only reacts once this render has committed,
// and the render in between would still see a flag left true by the
// previous injection and mount `loading` regardless of the delay. It
// takes `loadingDelay <= 0` rather than a plain false for the reason the
// initial state does - the default would otherwise lose a frame to a
// re-injection.
/* eslint-disable @eslint-react/set-state-in-effect */
setHasError(false)
setIsLoading(true)
setHasLoadingDelayElapsed(loadingDelayRef.current <= 0)
if (hasInjectedRef.current) {
setInjectionId((id) => id + 1)
} else {
hasInjectedRef.current = true
}
/* eslint-enable @eslint-react/set-state-in-effect */

let nonReactTarget: WrapperType
Expand Down Expand Up @@ -232,6 +278,38 @@ export const ReactSVG: React.ForwardRefExoticComponent<
wrapper,
])

// Keyed on `injectionId` rather than living in the injection effect, so
// that changing `loadingDelay` restarts the timer without re-running the
// injection. Only the timer lives here; a re-injection's flag is cleared by
// the injection effect itself, which is a render earlier than this can run.
//
// `isLoading` is a dependency so the timer is cleared once an injection
// finishes, and `injectionId` so it re-arms for every injection - including
// one that starts while the previous request is still in flight, which
// leaves `isLoading` true the whole way through.
React.useEffect(() => {
if (!isLoading) {
return
}

/* eslint-disable @eslint-react/set-state-in-effect */
if (loadingDelay <= 0) {
setHasLoadingDelayElapsed(true)
return
}

setHasLoadingDelayElapsed(false)
/* eslint-enable @eslint-react/set-state-in-effect */

const timeoutId = setTimeout(() => {
setHasLoadingDelayElapsed(true)
}, loadingDelay)

return () => {
clearTimeout(timeoutId)
}
}, [injectionId, isLoading, loadingDelay])

const Wrapper = wrapper

return (
Expand All @@ -245,7 +323,7 @@ export const ReactSVG: React.ForwardRefExoticComponent<
}
: {})}
>
{isLoading && Loading && <Loading />}
{isLoading && hasLoadingDelayElapsed && Loading && <Loading />}
{hasError && Fallback && <Fallback />}
</Wrapper>
)
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ interface BaseProps {
fallback?: React.ElementType
httpRequestWithCredentials?: boolean
loading?: React.ElementType
loadingDelay?: number
onError?: (error: unknown) => void
renumerateIRIElements?: boolean
src: string
Expand Down
Loading
Loading