Give createResource the source type it needs - #997
Merged
Conversation
`createResource(source, fetcher)` takes two non-defaulted type parameters, `T` and `S`. TypeScript has no partial type argument inference, so a call that supplies only `T` cannot select that overload at all: it falls through to the `(fetcher, options)` overload, which needs only `T`, and the call is reinterpreted with the source accessor standing in for the fetcher. Passing `S` as well selects the intended overload. `S` is written as `ReturnType<typeof <source>>` rather than a copy of the fetch function's parameter annotation: it is exact, it stays in step with the source memo, and it supplies the contextual type at the sites whose fetcher is an inline arrow with no annotation to copy. Dropping the explicit `T` instead and letting inference run would also silence the errors, but most of these fetchers return `httpGet(...).then((resp) => resp?.data)` and axios types `.data` as `any`, so `T` would infer as `any` and the resource would lose its type rather than regain it. Verified by assigning each resource to an incompatible type and reading what the compiler reports: with `T` inferred it reports nothing, because `any` is assignable to anything. 22 call sites. The resources now resolve to the types they always claimed, for example `JsonUsage | null | undefined` in `BillingPanel` and `JsonPlot | undefined` in `Pinned`.
Four call sites named a result type their fetch function never returns, so pinning the source type argument was not enough to select the `(source, fetcher)` overload. `getAuthUser`, `acceptInvite`, and `claimProject` navigate and report through `navigateNotify`; every path returns `null` or nothing, never a `JsonAuthUser`, `JsonAuthAck`, or `JsonOrganization`. All three resources are already discarded at the binding, so the declared type was decorative. They are now `void | null`, and the two type imports that only the old declarations used are dropped. `getKey` returns a bare `return;` on its guard paths, which is `void` rather than `undefined`, so the declared `undefined | JsonProjectKey | JsonProjectKeyCreated` never matched. Widened to `void`. `void` in a union reads as a mistake often enough that Biome flags it, so each one carries a suppression saying which function produces it.
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.
astro checkreports 806 errors. This takes it to 754, below the 758 it sat atbefore the Astro 7 upgrade.
What was actually wrong
createResourceis called in two shapes. The(source, fetcher)shape wasbeing given a single explicit type argument:
That call has always been wrong. It reproduces under the pre-upgrade toolchain,
so the upgrade did not break these call sites; it changed what happens after
they fail. solid-js 1.9.14 added
I = Tto the(fetcher, options)overload,whose return type became
InitializedResourceReturn<T | I, R>. When resolutionfalls through to it,
Tis left unresolved and leaks into the resource type, soa single bad call now also produces errors at every consumer of that resource.
That cascade is what this removes.
The fix names the source type as well, which lets the intended overload match:
ReturnType<typeof fetcher>rather than the object type spelled out, because itstays in step with the source memo and supplies a contextual type at sites whose
fetcher is an inline arrow with nothing to copy from.
Why not just drop the type argument
Removing the explicit type argument also silences the errors, and it is the
wrong fix. These fetchers end in
httpGet(...).then((resp) => resp?.data), and.dataisany, so inference resolves the whole resource toany. Bothapproaches reach zero errors, so error counts cannot tell them apart. Two
independent probes confirmed the difference: under inference the resource
assigns to a deliberately incompatible type with no complaint, which only
anydoes, while naming the source type yields
JsonUsage | null | undefined.Trading a loud error for a silent
anywould have made this worse, not better.Four resources that were declared as something they never return
getAuthUser,acceptInvite, andclaimProjectnavigate; they returnvoid | null, neverJsonAuthUser,JsonAuthAck, orJsonOrganization. Allthree resources are discarded at the binding.
getKeyhas a barereturn, soit is
void, notundefined. Naming the source type is not enough at thesefour, because the declared
Twas the lie. Each carries a suppression namingthe function, matching the convention already used in 24 other files.
Verification
astro check806 to 754. 58 errors removed, 6 added.positions, relabelled because the resource finally resolves, and the other 3
are latent problems that correct resolution stops masking.
any. All 26 edited sites resolve to concrete types, and theimplicit-any count drops from 219 to 214.
JavaScript, so the reindentation carries no behavior.
biome ciexits 0 at the same 39 warnings as before,vitestpasses, and theCloudflare build path succeeds.
Left alone
15 call sites still fail. Each needs a real typing decision rather than a type
argument: 11 where the source memo is genuinely looser than the fetcher's
parameter annotation, and 4 where the declared type is wrong but the resource is
consumed, so an honest type would push errors into consumers. Those are runtime
changes and do not belong in a types-only change.