-
Notifications
You must be signed in to change notification settings - Fork 10
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
Simplify converting EVM addresses to oasis1 #1365
Conversation
Deployed to Cloudflare Pages
|
@@ -104,7 +103,6 @@ export const routes: RouteObject[] = [ | |||
{ | |||
path: '/search', // Global search | |||
element: withDefaultTheme(<SearchResultsPage />), | |||
loader: searchParamLoader, |
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 are we removing the searchParamLoader
here? I would rather see that app fails in a loader if invalid URL, instead of in component.
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.
I don't think searchParamLoader was validating the URL. We just needed to parse the query in URL, but async code forced us to use a loader (or useEffects)
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.
explorer/src/app/components/Search/search-utils.ts
Lines 132 to 136 in 14de628
const network = useNetworkParam() | |
if (!!network && !RouteUtils.getEnabledNetworks().includes(network)) { | |
throw new AppError(AppErrors.InvalidUrl) | |
} |
To me it seems like it throws, if the scope isn't specified.
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.
Should have linked to master, either way, this code was previously in searchParamLoader
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.
Checked more thoroughly
Before:
- /:_network/:_layer/search:
- /:_network/:_layer/: assertEnabledScope validated network and scope
- /:_network/:_layer/search: searchParamLoader validated network again
- /search: searchParamLoader skipped the validation because !!network is false
After:
- /:_network/:_layer/search:
- /:_network/:_layer/: assertEnabledScope validates network and scope
- /search: nothing to validate
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.
(found #1366 while checking)
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.
LGTM, just fix the linting issues
14de628
to
3d80baf
Compare
No description provided.