refactor: replace @ts-expect-error with typed assertions in hook system#24
Open
glacierphonk wants to merge 1 commit intogramiojs:mainfrom
Open
refactor: replace @ts-expect-error with typed assertions in hook system#24glacierphonk wants to merge 1 commit intogramiojs:mainfrom
glacierphonk wants to merge 1 commit intogramiojs:mainfrom
Conversation
Remove 15 @ts-expect-error suppressions and 7 TODO comments from the hook system by using targeted type assertions: - runImmutableHooks: cast hooks array to match correlated context params, resolving the "solve that later" TODO - preRequest/onResponse/onResponseError/onApiCall registration: cast methods array to readonly string[] for .includes() checks, cast context to the narrowed handler type, and cast wrapper functions to their store types - _callApi onResponse: cast the response object to the hook's expected parameter type, also removing the as-any on data.result - onError: cast errContext after the .is() runtime guard Each cast is sound because the runtime logic (method filtering, .is() narrowing, type parameter correlation) guarantees the types match. Explicit casts are preferable to @ts-expect-error because they only suppress the specific expression and won't mask future unrelated type errors on the same line.
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.
Summary
Removes 15
@ts-expect-errorsuppressions and 7 TODO comments from the hook system insrc/bot.tsby replacing them with targeted type assertions.Changes by area
runImmutableHooks(was://TODO: solve that later)typeparameter guarantees the hook and args match at runtime.Hook registration methods (
preRequest,onResponse,onResponseError,onApiCall)methods.includes(context.method): Castmethodstoreadonly string[]so.includes()accepts the widerkeyof APIMethodsvalue. The runtime guard ensures only matching methods reach the handler.handler(context): Cast context to the handler's narrowed parameter type (e.g.,Hooks.PreRequestContext<Methods>). Safe because the.includes()guard above guarantees the method matches.as Hooks.PreRequest). The wrapper accepts the wide type and delegates to the narrow handler when matched.methodsOrHandlerto the store type. TypeScript can't fully narrow the generic union throughtypeof/Array.isArrayguards, but the overload signatures guarantee this is the function type._callApionResponse hook call (was:// TODO: fix type error)Parameters<Hooks.OnResponse>[0]. Also removed theas anycast ondata.resultthat was previously needed.onErrorhandler (was:// TODO: Sorry... fix later)errContextto the handler's expected parameter type after the.is()runtime guard narrows the context.onResponseErrorwrapper — also forwards theapiparameter to the filtered handler (it was silently dropped in the original wrapper).Why casts over
@ts-expect-error@ts-expect-errorsuppresses the entire next line, masking any future unrelated type errors. Explicitascasts only affect the specific expression and document exactly which type boundary is being crossed. They also produce better IDE hover information.Test plan
bun test— all 208 tests passbun run type— clean (tsc --noEmit)bun run lint— no new warnings (all warnings are pre-existing in other files)