feat: sync chains with url #398

Xaroz · 2025-01-23T19:38:10Z

Sync chains select with URL so that there is persistence
Sync token with URL for persistence
Added util functions to handle special cases so that app does not crash

fixes #397

vercel · 2025-01-23T19:38:27Z

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

Name	Status	Preview	Comments	Updated (UTC)
hyperlane-warp-template	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 26, 2025 7:40pm

5 Skipped Deployments

Name	Status	Preview	Updated (UTC)
injective-bridge	⬜️ Ignored (Inspect)		Jan 26, 2025 7:40pm
nautilus-bridge	⬜️ Ignored (Inspect)		Jan 26, 2025 7:40pm
nexus-bridge	⬜️ Ignored (Inspect)		Jan 26, 2025 7:40pm
renzo-bridge	⬜️ Ignored (Inspect)	Visit Preview	Jan 26, 2025 7:40pm
trump-bridge	⬜️ Ignored (Inspect)	Visit Preview	Jan 26, 2025 7:40pm

Xaroz · 2025-01-23T19:39:00Z

src/features/transfer/TransferTokenForm.tsx

  return useMemo(() => {
    const firstToken = warpCore.tokens[0];
    const connectedToken = firstToken.connections?.[0];
+
+    if (!isReady) return null;


router needs to be ready before we can actually check if any of the URL queries are available, if this is not done, it will override the URL query using the fallback

Just curious, why does it need to be ready? Does the URL not have the params in it regardless of the router status?

Also what happens if you return the default values here instead of null (i.e. the ones that we use when the param isn't set)?

removed dependency from next router and instead default to browser native URL handling

vercel · 2025-01-23T20:37:01Z

Deployment failed with the following error:

Preview deployments are disabled for this project.

src/features/tokens/TokenSelectField.tsx

jmrossy · 2025-01-24T14:23:30Z

src/features/transfer/TransferTokenForm.tsx

  return useMemo(() => {
    const firstToken = warpCore.tokens[0];
    const connectedToken = firstToken.connections?.[0];
+
+    if (!isReady) return null;


Just curious, why does it need to be ready? Does the URL not have the params in it regardless of the router status?

jmrossy

Your general approach to solving the problem here is good but there IMO there are some subtle code smells here (see comments). These aren't obvious issues, just subtleties related to code quality and organization that may possibly hurt maintainability in the long run. But don't be discouraged. You'll naturally get better at nuances like naming, organization, and consistency with more experience.

jmrossy · 2025-01-25T13:47:16Z

src/consts/app.ts

@@ -13,3 +13,9 @@ export const APP_URL = 'hyperlane-warp-template.vercel.app';
 export const BRAND_COLOR = Color.primary['500'];
 export const BACKGROUND_COLOR = Color.primary['500'];
 export const BACKGROUND_IMAGE = 'url(/backgrounds/main.svg)';
+
+export enum WARP_QUERY_PARAMS {


Move this somewhere else. These are consts that we expect people to change to customize the app. Consts for core functionality should probably be somewhere else

Moved under a new file inside const called core.ts, I was debating to put it in common.ts too, but felt like it didn't really match what the enum meant to do

jmrossy · 2025-01-25T13:48:42Z

src/features/chains/utils.ts

+/**
+ * Check if given chainName has valid chain metadata and return chainName if chain is valid
+ */
+export function getValidChain(


Your former isValidChain was more clear IMO

The reason for this change is that I want to make sure to get the chainName or undefined if the chain does not exist, instead of a boolean, but I am happy to make adjustments if needed

jmrossy · 2025-01-25T13:51:15Z

src/features/transfer/TransferTokenForm.tsx

+  handleSwapChain,
+}: {
+  disabled?: boolean;
+  handleSwapChain(origin: string, destination: string): void;


I don't see this syntax often. To clarify, is this equivalent to:
handleSwapChain: (origin: string, destination: string)=> void?

Also nit but I think onSwapChain would be more consistent with the nomenclature we use elsewhere

Kinda derped here, fixed

jmrossy · 2025-01-25T13:54:54Z

src/features/transfer/TransferTokenForm.tsx

+    updateQueryParam(WARP_QUERY_PARAMS.ORIGIN, origin);
+    updateQueryParam(WARP_QUERY_PARAMS.DESTINATION, destination);


For the TokenSelectField, you handle the query param setting in the component. But for these others, you're doing it in TransferTokenForm. Let's be consistent to keep code cohesion high

Moved the logic inside TransferTokenForm for consistency

jmrossy · 2025-01-25T13:57:01Z

src/features/transfer/TransferTokenForm.tsx

+  const handleChange = (chainName: string, fieldName: string) => {
+    if (fieldName === 'origin') handleGetTokensRoute(chainName, values.destination);
+    else handleGetTokensRoute(values.origin, chainName);
+    updateQueryParam(fieldName, chainName);


You've already decided to make a separate WARP_QUERY_PARAMS enum which defines the param name, but here you're assuming that fieldName === WARP_QUERY_PARAMS. If you can assume they're always the same, remove WARP_QUERY_PARAMS. If you can't, use WARP_QUERY_PARAMS

done, did a check for fieldName === WARP_QUERY_PARAMS

src/features/transfer/TransferTokenForm.tsx

jmrossy · 2025-01-25T14:01:45Z

src/features/transfer/TransferTokenForm.tsx

@@ -449,17 +492,36 @@ function WarningBanners() {

 function useFormInitialValues(): TransferFormValues {
  const warpCore = useWarpCore();
+  const parameters = new URLSearchParams(window.location.search);


Consider maybe moving the interactions with URLSearchParams to your query params file since you're already doing that with the update function and it's better to be consistent. When you introduce a layer of abstraction, it's better to always use it as much as possible instead of creating two paths to the same thing.

created a function called getQueryParams for this, also use this function in the updateQueryParams function

feat: sync chains with url

70ac191

Xaroz requested review from jmrossy and nambrot as code owners January 23, 2025 19:38

Xaroz commented Jan 23, 2025

View reviewed changes

vercel bot deployed to Preview – hyperlane-warp-template January 23, 2025 19:44 View deployment

feat: add token persistence and utils functions

06d71ab

vercel bot deployed to Preview – hyperlane-warp-template January 23, 2025 20:44 View deployment

jmrossy reviewed Jan 24, 2025

View reviewed changes

Xaroz added 2 commits January 24, 2025 15:39

feat: update status machine to use browser defaults

6eb7689

chore: clean up

9d5c079

vercel bot had a problem deploying to Preview – hyperlane-warp-template January 24, 2025 19:43 Failure

chore: remove unused util function

dd13742

Xaroz requested a review from jmrossy January 24, 2025 19:48

vercel bot had a problem deploying to Preview – hyperlane-warp-template January 24, 2025 19:52 Failure

fix: cycle dependency

8818f75

vercel bot deployed to Preview – hyperlane-warp-template January 24, 2025 20:42 View deployment

jmrossy reviewed Jan 25, 2025

View reviewed changes

chore: PR recommendations

e2b9a55

vercel bot deployed to Preview – hyperlane-warp-template January 26, 2025 19:40 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: sync chains with url #398

feat: sync chains with url #398

Xaroz commented Jan 23, 2025 •

edited

Loading

vercel bot commented Jan 23, 2025 •

edited

Loading

Xaroz Jan 23, 2025 •

edited

Loading

jmrossy Jan 24, 2025

jmrossy Jan 24, 2025

Xaroz Jan 24, 2025

vercel bot commented Jan 23, 2025

jmrossy Jan 24, 2025

jmrossy left a comment

jmrossy Jan 25, 2025

Xaroz Jan 26, 2025

jmrossy Jan 25, 2025

Xaroz Jan 26, 2025

jmrossy Jan 25, 2025

jmrossy Jan 25, 2025

Xaroz Jan 26, 2025

jmrossy Jan 25, 2025

Xaroz Jan 26, 2025

jmrossy Jan 25, 2025

Xaroz Jan 26, 2025 •

edited

Loading

jmrossy Jan 25, 2025 •

edited

Loading

Xaroz Jan 26, 2025 •

edited

Loading

		updateQueryParam(WARP_QUERY_PARAMS.ORIGIN, origin);
		updateQueryParam(WARP_QUERY_PARAMS.DESTINATION, destination);

feat: sync chains with url #398

Are you sure you want to change the base?

feat: sync chains with url #398

Conversation

Xaroz commented Jan 23, 2025 • edited Loading

vercel bot commented Jan 23, 2025 • edited Loading

Xaroz Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Jan 23, 2025

Choose a reason for hiding this comment

jmrossy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xaroz Jan 26, 2025 • edited Loading

Choose a reason for hiding this comment

jmrossy Jan 25, 2025 • edited Loading

Choose a reason for hiding this comment

Xaroz Jan 26, 2025 • edited Loading

Choose a reason for hiding this comment

Xaroz commented Jan 23, 2025 •

edited

Loading

vercel bot commented Jan 23, 2025 •

edited

Loading

Xaroz Jan 23, 2025 •

edited

Loading

Xaroz Jan 26, 2025 •

edited

Loading

jmrossy Jan 25, 2025 •

edited

Loading

Xaroz Jan 26, 2025 •

edited

Loading