test: stop Iconify's API query from firing into a torn-down jsdom - #202
Merged
Conversation
CI failed intermittently with every test passing:
Test Files 519 passed (519)
Tests 3977 passed (3977)
Errors 1 error
ReferenceError: window is not defined
at cleanup @iconify/react/dist/iconify.js:1833
Rendering an <Icon> whose icon is not already in a loaded collection makes
Iconify query its public API, and that query arms retry/timeout timers —
measured here as 750ms, 499ms and 4000ms, armed one tick after render rather
than during it. A test file that finishes inside 4s leaves the last one live;
when it fires, jsdom is gone, so Iconify's setState reaches react-dom, which
touches `window` and throws. Vitest counts that as an unhandled error and
fails the run. Whether it happens depends only on how long the run takes,
which is why it never reproduced on a single file.
It also meant the suite was reaching api.iconify.design on every icon render.
An inert API module in a new setup file creates no query, so no timers are
armed and nothing leaves the machine. This matches the app, where
`preloadIcons()` registers every collection from bundled packages and the API
is never used. Rendered output does not change: icons already came out as an
empty <span> in tests, because the query never resolved in time anyway.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure
CI has been failing intermittently with every test passing:
Most recently on #199, where a re-run cleared it with no code change.
Root cause
Rendering an
<Icon>whose icon is not already in a loaded collection makes Iconify query its public API. That query arms retry/timeout timers — measured in this repo as 750ms, 499ms and 4000ms — and it arms them one tick after the render, not during it.A test file that finishes inside 4s leaves the last timer live. When it fires, jsdom has been torn down, so Iconify's
setStatereaches react-dom, which toucheswindowand throws. Vitest counts that as an unhandled error and fails the run even though nothing actually failed.Whether it happens depends only on how long the run takes, which is why it never reproduced on a single file —
src/plugins/docker/components/PortChips.test.tsx, the file CI blamed, passes cleanly five runs in a row on its own. The blamed file is just whichever one happened to be running when a stray timer fired.A side effect worth noting: the suite has been reaching
api.iconify.designon every icon render.Fix
A new
vitest.setup.tsinstalls an inert Iconify API module, so no query is ever created, no timers are armed, and nothing leaves the machine.This matches the app:
preloadIcons()registers every collection from bundled packages, and the API is never used in production either. Rendered output does not change — icons already came out as an empty<span>in tests, because the query never resolved in time anyway.src/utils/iconifyTestApi.test.tsxguards it.Verification
expected [ 750, 499, 4000 ] to deeply equal []; with it, it passes.tsc --noEmitclean.Note
This does not make icons render in tests — it makes them deterministically not render, which is what they already did. Loading the real collections in the setup file would render them for real, but it changes the DOM every icon-bearing test sees, so it is deliberately not part of this change.