-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spike [handlers]: explicit relationship between query and buildCallData signatures #196
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
lib/modules/pool/actions/add-liquidity/queries/useAddLiquidityPreviewQuery.ts
Outdated
Show resolved
Hide resolved
const isSdkHandler = 'sdkQueryOutput' in queryOutput && queryOutput.sdkQueryOutput | ||
|
||
if (isSdkHandler) { | ||
const sdkBuildInput: SdkBuildAddLiquidityInputs = { | ||
...baseInput, | ||
sdkQueryOutput: queryOutput.sdkQueryOutput, | ||
} | ||
return handler.buildAddLiquidityCallData(sdkBuildInput) | ||
} | ||
if (handler instanceof TwammAddLiquidityHandler) { | ||
return handler.buildAddLiquidityCallData(baseInput) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pass the whole queryOutput
into the handler build call on the handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like it doesn't make sense to do this conditional deconstruction in this query. The handler should just know what it's getting because of the type, shouldn't it?
Closed by #211 |
Description
Refactors
AddLiquidityHandler
interface to depend on generic TS types.The new code is more verbose but also more explicit and type-safe.
Now we can have
Default SDK handlers where:
SdkQueryAddLiquidityOutput
that contains a mandatorysdkQueryOutput: AddLiquidityQueryOutput
fieldEdge case handlers (added a fake Twamm handler example )where:
sdkQueryOutput: AddLiquidityQueryOutput
fieldbptOut
fieldType of change
expected)
How should this be tested?
Please provide instructions so we can test. Please also list any relevant details for your test
configuration.
Visual context
Please provide any relevant visual context for UI changes or additions. This could be static
screenshots or a loom screencast.
Checklist: