improve: allow canonical linking in deep-link

[james-a-morris]

Goal: Allow a user to supply a canonical chain name in a bridge URL.

Test Case: Visit: {{deployment-url}}/?from=mainnet&to=optimism&asset=usdc&amount=100

References #290

[vercel]

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

Name	Status	Preview	Updated
frontend-v2	✅ Ready (Inspect)	Visit Preview	Oct 7, 2022 at 11:02PM (UTC)
goerli-frontend-v2	✅ Ready (Inspect)	Visit Preview	Oct 7, 2022 at 11:02PM (UTC)

[daywiss]

surprised u have to cast this, whats the error if you set the variable to this type? CanonicalChainName:Record<string, number>=...

[james-a-morris]

There isn't a need; that's a good point!

I've updated the code to remove the cast so that TS can type the object as:

{
    [k: string]: number;
}

[daywiss]

if chainIdOrCanonical does not exist this ends up returning NaN. IMO i would just throw errors with invalid inputs or something that doesnt exist and handle that upstream, but i realize that is annoying in react

[james-a-morris]

I agree with this. The only drawback is that the calling function expects a number (potentially NaN) and doesn't handle its error logging.

This can be seen in the base case if no queries are passed, the to/from is undefined (by visiting https://across.to/ or http://localhost:3000/ on local).

My understanding is that these hooks need to fail silently because the NaN resolves as invalid. Here's a snippet from the initial calling code before this PRs edits:

https://github.com/across-protocol/frontend-v2/blob/5f1ceabe45d5dbd5f2bbabadd5bf82c7516133f9/src/hooks/useSendForm.tsx#L376-L381

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Pages

Latest commit:	f10db64
Status:	🚫  Build failed.

View logs

[daywiss]

lgtm

[dohaki]

LGTM 👍

[nicholaspai]

Have a comment on the conditional branching

[nicholaspai]

It might be better to use isNaN(chainIdOrCanonical) to first see if the string is a number. In which case we can parse as number.

If that fails then it must be a canonical name or a failure condition occurs.

I think the way you have it now, you're likely to try to parse invalid canonical chain names or empty strings as Number and that might give issues

[james-a-morris]

That's a good point!

I was trying to keep this function's logic as a stand-in for the original logic, which converts the to/from params to numbers directly by adding Number(...).

I will update it now and look for other improvements 😃

[james-a-morris]

I've updated the function first to check if the chainIdOrCanonical variable is a numeric string. I've added doc comments to define how the function should return a NaN if an invalid case is found (non-numeric string & string does not contain a canonical name).
https://github.com/across-protocol/frontend-v2/blob/e0d9671544967f277f21f00f9812db2e7738b942/src/utils/config.ts#L153-L164