-
Notifications
You must be signed in to change notification settings - Fork 12
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
fix: redirect page and first-hit normalization #201
Conversation
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.
self review
window.location.replace(windowLocation.href) | ||
} | ||
|
||
return (<>First-hit on IPFS hosted service-worker-gateway. Reloading</>) |
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.
we could probably add a spinner or something better here.
/** | ||
* If you host helia-service-worker-gateway on an IPFS domain, the redirects file will route some requests from | ||
* `<domain>/<wildcard-splat>` to `https://<domain>/?helia-sw=<wildcard-splat>`. | ||
* | ||
* This function will check for "?helia-sw=" in the URL and modify the URL so that it works with the rest of our logic | ||
*/ | ||
function translateIpfsRedirectUrl (urlString: string): URL { | ||
const url = new URL(urlString) | ||
const heliaSw = url.searchParams.get('helia-sw') | ||
if (heliaSw != null) { | ||
url.searchParams.delete('helia-sw') | ||
url.pathname = heliaSw | ||
} | ||
return url | ||
} |
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.
this was moved from ipfs-hosted-redirect-utils.ts without modification
|
||
const routes: Route[] = [ | ||
{ default: true, component: LazyHelperUi }, | ||
{ path: '#/ipfs-sw-config', shouldRender: async () => (await import('./lib/routing-render-checks')).shouldRenderConfigPage(), component: LazyConfig }, | ||
{ shouldRender: async () => (await import('./lib/routing-render-checks')).shouldRenderRedirectPage(), component: LazyRedirectPage } | ||
{ shouldRender: async () => (await import('./lib/routing-render-checks.js')).shouldRenderRedirectsInterstitial(), component: LazyInterstitial }, |
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.
We could probably import this check function immediately since it's tiny but this is consistent at least.
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.
Sounds like a reasonable way to avoid an additional round trip.
I need this in to fix config page loading on subdomains on localhost... will merge shortly. |
Nice! From my testing this resolves the redirection loop. |
Title
fix: redirect page and first-hit normalization
Description
Fixes config page loading and normalizes first hits landing on redirectPage.
Fixes #148
Fixes #150
Summary of changes
Notes & open questions
This normalizes how redirects are handled. Now, the first hit on a redirect page will always land on the redirect page, and subsequent hits will be redirected to the target page. Previously, the first hit for ?helia-sw would actually load the helperUI page, which is not ideal.
Change checklist