Add a loadingDelay prop

[tanem]

Adds loadingDelay, milliseconds to wait before rendering loading. Default 0, so this is purely additive and changes nothing for anyone who does not set it. An injection that finishes sooner than the delay never renders loading at all.

Why

The flash this suppresses is not new, and it is not about the request cache. A 30ms cold load — localhost, a warm CDN edge, a file:// read — paints a loading element and pulls it away again. NN/g's position on progress indicators is that a sub-second one is worse than none, because the user cannot keep up with what happened and may feel anxious about whatever flashed. 200–300ms before revealing an indicator is the usual industry choice.

Measured in Chrome 151, six icons per case, against a real server:

case	load	loadingDelay	entered the DOM	median lifetime	frames showing a loader
A	cold 30ms	0	6/6	38.3ms	5 of 14
B	cold 30ms	200	0/6	—	0 of 26
C	cold 2500ms	200	6/6	2305.9ms	277 of 311
D	warm cache	200	0/6	—	0 of 9
E	warm cache	0	6/6	2.4ms	1 of 9

A is the case the prop exists for: the loader paints on 5 frames out of 14 and is gone. C shows the delay honoured to within ~6ms of the 200 asked for, so a genuinely slow load still gets its indicator. B and D suppress at the DOM level rather than the paint level, which is a stronger result than a paint-level A/B: an element never in the DOM cannot paint, and cannot be observed by assistive technology either.

Why the default is 0

This package cannot tell a feedback loading component from a layout reservation one. A spinner benefits from a delay; a skeleton sized to hold the SVG's space exists to keep Cumulative Layout Shift down, and delaying that turns one layout shift into two. Only the consumer knows which they wrote, so the prop is opt-in.

An earlier framing of this rested on a screen-reader argument — that a briefly-mounted loading element might announce. That was measured separately and reported no: VoiceOver ignores any live region inserted already-populated, which is how React mounts a loading component, so lifetime never mattered. This lands on the flash alone.

Notes on the implementation

• The delay is its own effect keyed on isLoading, not a dependency of the injection effect. Adding loadingDelay to that effect's dependency array would make changing the delay re-run the injection. Keyed this way, the injection effect setting isLoading back to true is what restarts the delay for a re-injection.
• The initial state reads the prop (useState(loadingDelay <= 0)) rather than starting false. useEffect runs after paint, so starting false would cost the default one frame without the loader — a regression in exactly the path this must leave alone. The 22 existing snapshots passing unchanged is the check on that.
• <= 0, not === 0. A negative delay would otherwise reach setTimeout, which clamps it to 0 and fires in a later task, holding the loader back for a caller who asked for no delay.
• fallback is not delayed. An error costs a round trip that no cache short-circuits, so there is no flash to suppress and delaying the message is a straight downgrade. Stated in the README rather than left implicit, and pinned by a test.

Tests

Four, written before the change. Two were red for the right reason — the loader present when it should have been held back. The other two are pins on paths that must not move: loadingDelay={0} renders immediately, and fallback is not delayed. Both passed before the change, which is the point of them.

npm test is green: 46 tests, 100% statement coverage on ReactSVG.tsx, all seven React versions (16.8, 16.14, 17.0, 18.0, 18.3, 19.0, 19.1). size-limit reports 1.76 kB ESM against the 2 kB budget and 2.22 kB CJS against 2.5 kB — roughly 100 bytes gzipped for the extra state and effect, so no budget change.

No example added: examples/loading pins react-svg: "latest", so it cannot demonstrate an unreleased prop. That is a follow-up for after the release.

🤖 Generated with Claude Code

[tanem]

Code review

Found 4 issues:

1. On re-injection, loading mounts regardless of the delay, and can visibly flash. hasLoadingDelayElapsed is reset by a separate effect keyed on isLoading, so it lags one render behind the injection effect setting isLoading back to true — the render in between sees the stale true and mounts Loading. Measured in Chrome against dist/ built from this branch: after a completed injection, changing src with loadingDelay={5000} mounted the loader in 60/60 runs, and a frame was painted with it on screen in 8/60. Mount and unmount landed in different tasks in all 60, so there is always a paint opportunity; the runs that painted are those where the loader stayed mounted longest (1.0–2.0ms vs 0.1ms floor), so slower devices and busier main threads should hit it more often. That is the flash the prop exists to suppress, and it contradicts the README's "an injection that finishes sooner than the delay never renders loading at all". The comment at L247-248 asserts this case is handled.

react-svg/src/ReactSVG.tsx

Lines 244 to 270 in 327a92c

	// Keyed on `isLoading` rather than living in the injection effect, so that
	// changing `loadingDelay` restarts the timer without re-running the
	// injection. The injection effect setting `isLoading` back to true is what
	// restarts the delay for a re-injection.
	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)
	}
	}, [isLoading, loadingDelay])

2. Three new tests reuse faker.seed() values already used later in the same file, so they resolve to the same src as existing tests and share svg-injector's cache (AGENTS.md says "Give each test its own faker.seed() and a faker.string.uuid() SVG URL, or svg-injector's cache leaks state between tests"). Seed 133 collides with should allow modification of the SVG via the beforeInjection callback (L409), 134 with should render correctly when bypassing the request cache (L435), and 135 with should render correctly with an extensionless svg (L461). Same seed, same single faker.string.uuid() call, same UUID.

react-svg/test/browser.spec.tsx

Lines 277 to 279 in 327a92c

	faker.seed(133)
	const uuid = faker.string.uuid()

3. The manual screen-reader harness wasn't re-run or recorded, though this deliberately changes the loading element's lifecycle (AGENTS.md says "Run it and record the result in the PR when you change the ARIA wiring or the loading element's lifecycle, and update its recorded run in the same commit as any deliberate change to either"). loading now mounts late or not at all, but test/manual/ is untouched and its "Last run" table still records the 2026-08-04 run that predates the prop. The PR body says the screen-reader question "was measured separately" — AGENTS.md asks for that recorded in the same commit. Worth re-running after issue 1 is fixed, since the transient mount is exactly the kind of thing the harness exists to check.

react-svg/src/ReactSVG.tsx

Lines 284 to 286 in 327a92c

	>
	{isLoading && hasLoadingDelayElapsed && Loading && <Loading />}
	{hasError && Fallback && <Fallback />}

4. Three lines of the commit body on 327a92c are 73 columns (AGENTS.md says "Hard-wrap commit message bodies at 72 columns; git log does not reflow them"): "from a skeleton sized to reserve the SVG's space, and delaying the latter", "The delay is its own effect keyed on isLoading rather than a dependency", and "of the injection effect, so changing it does not re-inject. Initial state".

327a92c

Updated: issue 1 was originally posted as "no paint observed in jsdom". That measurement was unsound — jsdom has no paint layer, as AGENTS.md notes. Re-measured in real Chrome with rAF sampling, which shows it does paint.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[tanem]

Three of the four are addressed in eed92b7 and the amended 4cba345.

1. Re-injection flash — fixed. hasLoadingDelayElapsed is now cleared in the injection effect, in the same commit as setIsLoading(true), so no render sees a flag left true by the previous injection. loadingDelay is read through a ref like the callbacks above it, keeping it out of the injection effect's dependency list so changing the delay still doesn't re-inject. It clears to loadingDelay <= 0 rather than plain false, for the same reason the initial state does — otherwise the default would lose a frame without the loader on every re-injection.

Re-measured with the same Chrome probe, sampling requestAnimationFrame:

	before	after
loader mounted despite loadingDelay={5000}	60/60	0/60
frame painted with the loader	8/60	0/60
arm-phase mounts (positive control)	—	60/60

The positive control is the part that makes the zero meaningful: each iteration first runs an injection with an 80ms delay against a 400ms response, where the loader must appear. It did, 60/60, while the measured phase recorded none — so the harness was still sensitive rather than blind.

The regression test counts renders of loading rather than querying the DOM, because the stale mount lasted 0.1–2.0ms — long enough for Chrome to paint, far too short to catch after the fact. It fails against the previous code with Expected: 0, Received: 1.

2. Seed collisions — fixed. The three new tests moved to 190/191/192; the existing tests keep 133/134/135, so no existing URL changed. No seed is now used twice in test/.

4. Commit body wrap — fixed by amending, hence the force-push. git diff 327a92c0 4cba3450 is empty: message only, tree untouched. Both bodies now wrap at 72.

3. Manual screen-reader harness — still outstanding, and now more relevant than when I flagged it: loading no longer mounts at all on a re-injection whose delay hasn't elapsed, so what the harness measures has changed since the 2026-08-04 run.

Verified locally on the pushed tree: test:src 47 passed with 22 snapshots unchanged and 100% coverage of ReactSVG.tsx, plus lint, check:types, check:format, and size within both budgets (1.79/2 kB, 2.24/2.5 kB). I did not run the full React matrix.

🤖 Generated with Claude Code

[tanem]

Code review

Re-reviewed the delta since the last round (327a92c0..b06a2cb4) rather than the whole PR. Found 1 issue, fixed in 1fcdc16.

1. The fix for issue 1 introduced a regression of its own. The delay effect was keyed on isLoading and loadingDelay. A re-injection that starts while the previous request is still in flight leaves isLoading true the whole way through, so when loadingDelay is held constant — the usual way to pass it — neither dependency moved and the effect never re-ran to arm a new timer.

react-svg/src/ReactSVG.tsx

Lines 261 to 289 in b06a2cb

	])
	// Keyed on `isLoading` 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.
	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)
	}
	}, [isLoading, loadingDelay])

Two ways it showed. If the first delay had elapsed, clearing the flag at L132 left nothing to set it again, so loading stayed suppressed for the whole of the second injection however slow it was — a regression against the pre-eed92b7b behaviour, where the flag stayed true and the loader stayed on screen. If the first delay had not elapsed, its timer survived and fired against the old start time, so loading appeared early.

Verified rather than argued: a test that mounts with loadingDelay={50} against a 1000ms response, waits for the loader, then swaps src mid-flight, passes against 327a92c0 and fails against b06a2cb4.

The remaining cells of the matrix were fine — re-inject after completion with the delay unchanged, re-inject mid-flight with the delay changed, and re-inject after an error all pass on b06a2cb4. The gap was one cell, and should not carry an elapsed loadingDelay into a re-injection couldn't see it because it moves both dependencies at once: it waits for the first injection to finish and changes the delay 50 → 2000.

The fix adds an injectionId counter bumped whenever an injection starts, giving the delay effect something that moves every time. It is skipped for the first injection so mount still settles in one render. Four tests now walk the rest of the matrix; two of them fail against b06a2cb4.

test:src 52 passed, 22 snapshots unchanged, 100% coverage of ReactSVG.tsx. lint, check:types, check:format clean. size 1.84/2 kB and 2.29/2.5 kB, about +50 bytes each, both budgets unchanged. Full React matrix not run.

The screen-reader harness was re-run against the fix and re-recorded in the same commit, since the loading element's lifecycle changed again. Announcement behaviour is unchanged across all three runs. It still has no step that swaps src on a live component, so it does not reach either re-injection path — noted in the recorded run rather than papered over.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}