Skip to content
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

refactor: make getFastBridges reusable across components #1056

Closed
wants to merge 8 commits into from

Conversation

brtkx
Copy link
Contributor

@brtkx brtkx commented Jul 26, 2023

Closes #1055

@vercel
Copy link

vercel bot commented Jul 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Nov 14, 2023 6:37pm

@cla-bot cla-bot bot added the cla-signed label Jul 26, 2023
case FastBridgeNames.Connext:
return `https://bridge.connext.network/${tokenSymbol}-from-${slugFrom}-to-${slugTo}?amount=${amount}`
case FastBridgeNames.Across:
return 'https://across.to/bridge'
Copy link
Contributor Author

@brtkx brtkx Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Across query params seem not to be working at all anymore.

case FastBridgeNames.Stargate:
return `https://stargate.finance/transfer?srcChain=${slugFrom}&dstChain=${slugTo}&srcToken=${tokenSymbol}`
case FastBridgeNames.Synapse:
return 'https://synapseprotocol.com/'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synapse doesn't use query params, but detects the currently connected network.

@brtkx brtkx marked this pull request as ready for review July 27, 2023 11:43
@brtkx brtkx marked this pull request as draft July 27, 2023 11:44
@brtkx brtkx marked this pull request as ready for review November 14, 2023 15:55
@fionnachan fionnachan self-requested a review November 14, 2023 16:36
}
}

export function getFastBridges<TransferType extends 'bridge' | 'swap'>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the point of having one function, but if we always know when we call which one we want, would not be better to have two function with clear naming? Unless we plan to add a new parameter later

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe with a common helper function, but we would only export getFastSwaps, getFastBridges

@fionnachan
Copy link
Member

obsolete

@fionnachan fionnachan closed this Nov 15, 2023
@fionnachan fionnachan deleted the refactor-getFastBridges branch November 15, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor getFastBridges
3 participants