Skip to content
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: handle helia-sw query from _redirects #67

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Feb 28, 2024

Title

Description

Fixes an issue where helia-service-worker-gateway will redirect <domain>/<requestedPath> to <domain>/?helia-sw=/<requestedPath> and then not be handled by the service-worker appropriately.

Notes & open questions

still debugging this. WIP

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@lidel
Copy link
Member

lidel commented Mar 1, 2024

@SgtPooki is this draft ready for review or needs more work? lmk if a second pair of eyes on this would be helpful.

@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 1, 2024

@lidel I just needed some time to do some testing

@SgtPooki SgtPooki changed the title fix: helia-sw query from _redirects works fix: handle helia-sw query from _redirects Mar 2, 2024
Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review

import Config from './components/config.tsx'
import { ConfigContext } from './context/config-context.tsx'
import HelperUi from './helper-ui.tsx'
import { isConfigPage } from './lib/is-config-page.ts'
import { isPathOrSubdomainRequest, findOriginIsolationRedirect } from './lib/path-or-subdomain.ts'
import { isPathOrSubdomainRequest } from './lib/path-or-subdomain.ts'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to service-worker-context

Comment on lines +1 to +14
/**
* @file This file contains the ServiceWorkerProvider component which is used to register the service worker,
* and provide the isServiceWorkerRegistered state to the rest of the app.
*
* URL / location logic dependent upon the service worker being registered should be handled here. Some examples of this are:
*
* Before the Service Worker is registered (e.g. first requests to root hosted domain or subdomains):
*
* 1. Being redirected from _redirects file to a ?helia-sw= url
* 2. The app is loaded because service worker is not yet registered, we need to reload the page so the service worker intercepts the request
*
* After the service worker is loaded. Usually any react code isn't loaded, but some edge cases are:
* 1. The page being loaded using some /ip[fn]s/<path> url, but subdomain isolation is supported, so we need to redirect to the isolated origin
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some explanations

Comment on lines +35 to +40
if (windowLocation.href !== window.location.href) {
/**
* We're at a domain with ?helia-sw=, we can reload the page so the service worker will
* capture the request
*/
window.location.replace(windowLocation.href)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check could be more explicit for future devs, but translateIpfsRedirectUrl should make understanding this easy.

Comment on lines +45 to +49
void findOriginIsolationRedirect(windowLocation).then((originRedirect) => {
if (originRedirect !== null) {
window.location.replace(originRedirect)
}
})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/?helia-sw= redirect will load path URL.. if not handled by service worker for whatever reason (it should be, 99.99% of the time) then this will redirect that path request to subdomain if supported.

@@ -32,7 +72,7 @@ export const ServiceWorkerProvider = ({ children }): JSX.Element => {
}
}
void doWork()
}, [])
}, [isServiceWorkerRegistered])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're dependent upon isServiceWorkerRegistered, so we need to execute this effect more than a single time now

@SgtPooki SgtPooki marked this pull request as ready for review March 2, 2024 23:07
@SgtPooki SgtPooki self-assigned this Mar 2, 2024
@SgtPooki SgtPooki merged commit cfd70a6 into main Mar 5, 2024
14 of 15 checks passed
@SgtPooki SgtPooki deleted the fix/ipfs-hosted-redirects branch March 5, 2024 06:06
@lidel lidel mentioned this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants