Tech: Routes refactoring #151
Conversation
| remixRoutes({ strict: true }), | ||
| mode === 'production' && compression({ algorithm: 'gzip', compressionOptions: { level: 9 } }), | ||
| mode === 'production' && compression({ algorithm: 'brotliCompress' }), | ||
| isTest(mode) ? react() : remix({ routes: r => createRoutesFromFolders(r) }), |
There was a problem hiding this comment.
Because src was eliminated, additional configuration is redundant
| @@ -0,0 +1,241 @@ | |||
| declare module "remix-routes" { | |||
There was a problem hiding this comment.
types for $path from remix-routes
According to the docs, it could be generated in watch mode but I didn't manage to make it run with correct types (nested types was skipped). Seems that in watch mode @remix-run/v1-route-convention doesn't provide correct routes that remix-routes expect to see.
During build process everything works just fine 🤷♂️
@johnthecat what to hear your thoughts
| query: ExportedQuery<import('../routes/onboarding/password').SearchParams>, | ||
| }; | ||
|
|
||
| "/onboarding/restore/:mnemonic": { |
There was a problem hiding this comment.
Is it safe to pass sensitive data in the path? Can it be the case that UI might show the path somewhere on the screen and thus compromising the mnemonic
There was a problem hiding this comment.
It shouldn't be visible easily since it's an iframe, but if we go to a network tab and see the request, you will see a mnemonic. So, still kinda unsafe way
| if (hideButtonTimeout) { | ||
| clearTimeout(hideButtonTimeout); |
There was a problem hiding this comment.
why do we need hideButtonTimeout?
| switch (e) { | ||
| case 'frame': | ||
| if (lottie.currentFrame === 0) { | ||
| timeoutId = setTimeout(() => lottie && lottie.pause(), lottie.getDuration()); |
There was a problem hiding this comment.
it should pause the animation before the gift starts to open (it's shaking first for ~3s) and continue when a user clicks on Claim
| })(); | ||
| if (!gifts) return; | ||
|
|
||
| getGiftsState(getGifts()!).then(([unclaimed]) => setUnclaimed(unclaimed)); |
There was a problem hiding this comment.
I think it's less readable when we call on a function inside another function with a symbol in the end as well
maybe it's better to separate them
| if (!mounted) return; | ||
|
|
||
| if (value) { | ||
| navigate($path('/onboarding/restore/:mnemonic', { mnemonic: value }), { replace: true }); |
There was a problem hiding this comment.
not sure if it's safe to pass it like this, we don't even give access to the mnemonic in localStarage only is SecretLocalStorage
| const navigate = useNavigate(); | ||
| const { assets } = useGlobalContext(); | ||
|
|
||
| const selectedAsset = pickAsset(chainId, assetId, assets); |
There was a problem hiding this comment.
after we come up with a state manager it might be more convenient to have selectedAsset there, otherwise, we need to run through all list of chain and asset every time
| }; | ||
| }, [amount, isAmountValid, maxAmountToSend, isPending, isAccountTerminate]); | ||
| const checkBalanceDeposit = !transferAll && +maxAmountToSend - (+amount || 0) < deposit; | ||
| setIsAmountValid(!!Number(+amount) && (+amount || 0) <= +maxAmountToSend && !checkBalanceDeposit); |
There was a problem hiding this comment.
we don't need both Number() and +amount
| // onAmountChange: () => { | ||
| // setIsAmountValid(prev => prev && !!deposit && +amount >= deposit); | ||
| // }, |
There was a problem hiding this comment.
this part was needed to check for gifts that a transfer amount was bigger than a deposit. Otherwise, a gift account cannot be created.
You can move this condition to another place since we no longer have a dependence on mainbatton
| query: ExportedQuery<import('../routes/onboarding/password').SearchParams>, | ||
| }; | ||
|
|
||
| "/onboarding/restore/:mnemonic": { |
There was a problem hiding this comment.
It shouldn't be visible easily since it's an iframe, but if we go to a network tab and see the request, you will see a mnemonic. So, still kinda unsafe way
srcfolder, made it more canonical in Remix world -appremix-routesgenerates types for routes inapp/types, save it in project's repository