-
Notifications
You must be signed in to change notification settings - Fork 13
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: no-sw error shows for subdomain requests #491
Changes from all commits
6a97ea8
cfe7ab4
5e7e9fb
71a622c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ if ('serviceWorker' in navigator) { | |
|
||
const routes: Route[] = [ | ||
{ default: true, component: ErrorPage ?? LazyHelperUi }, | ||
{ shouldRender: async () => renderChecks.shouldRenderNoServiceWorkerError(), component: LazyServiceWorkerErrorPage }, | ||
{ shouldRender: async () => renderChecks.shouldRenderRedirectsInterstitial(), component: LazyInterstitial }, | ||
Comment on lines
24
to
26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the actual bug fix.. |
||
{ path: '#/ipfs-sw-config', shouldRender: async () => renderChecks.shouldRenderConfigPage(), component: LazyConfig }, | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,10 @@ import { test as base, type Page } from '@playwright/test' | |
import { setConfig, setSubdomainConfig } from './set-sw-config.js' | ||
import { waitForServiceWorker } from './wait-for-service-worker.js' | ||
|
||
function isNoServiceWorkerProject <T extends typeof base = typeof base> (test: T): boolean { | ||
return test.info().project.name === 'no-service-worker' | ||
} | ||
|
||
const rootDomain = async ({ baseURL }, use): Promise<void> => { | ||
const url = new URL(baseURL) | ||
await use(url.host) | ||
|
@@ -13,13 +17,20 @@ const baseURLProtocol = async ({ baseURL }, use): Promise<void> => { | |
|
||
export const test = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
rootDomain: [rootDomain, { scope: 'test' }], | ||
protocol: [baseURLProtocol, { scope: 'test' }] | ||
protocol: [baseURLProtocol, { scope: 'test' }], | ||
page: async ({ page }, use) => { | ||
if (isNoServiceWorkerProject(test)) { | ||
test.skip() | ||
return | ||
} | ||
await use(page) | ||
} | ||
}) | ||
|
||
/** | ||
* You should use this fixture instead of the `test` fixture from `@playwright/test` when testing path routing via the service worker. | ||
*/ | ||
export const testPathRouting = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
export const testPathRouting = test.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idk why this wasn't being done before, but all of our custom test fixtures now extend from our |
||
rootDomain: [rootDomain, { scope: 'test' }], | ||
protocol: [baseURLProtocol, { scope: 'test' }], | ||
page: async ({ page, rootDomain }, use) => { | ||
|
@@ -63,7 +74,7 @@ export const testPathRouting = base.extend<{ rootDomain: string, baseURL: string | |
* }) | ||
* ``` | ||
*/ | ||
export const testSubdomainRouting = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
export const testSubdomainRouting = test.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
rootDomain: [rootDomain, { scope: 'test' }], | ||
protocol: [baseURLProtocol, { scope: 'test' }], | ||
page: async ({ page, baseURL }, use) => { | ||
|
@@ -108,4 +119,21 @@ export const testSubdomainRouting = base.extend<{ rootDomain: string, baseURL: s | |
} | ||
}) | ||
|
||
/** | ||
* A fixture that skips tests that require the service worker. This is needed in order to test handling of requests where the service worker is not present. | ||
* | ||
* @see https://github.com/ipfs/service-worker-gateway/issues/272 | ||
*/ | ||
export const testNoServiceWorker = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not extending our |
||
rootDomain: [rootDomain, { scope: 'test' }], | ||
protocol: [baseURLProtocol, { scope: 'test' }], | ||
page: async ({ page }, use) => { | ||
if (!isNoServiceWorkerProject(testNoServiceWorker)) { | ||
testNoServiceWorker.skip() | ||
return | ||
} | ||
await use(page) | ||
} | ||
}) | ||
|
||
export { expect } from '@playwright/test' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { testNoServiceWorker as test, expect } from './fixtures/config-test-fixtures.js' | ||
import { getNoServiceWorkerError } from './fixtures/locators.js' | ||
|
||
test.describe('no-service-worker', () => { | ||
test('Error renders on landing page', async ({ page }) => { | ||
await page.goto('/') | ||
|
||
await expect(getNoServiceWorkerError(page)).toBeVisible() | ||
}) | ||
|
||
test('Error renders on subdomain page', async ({ page, rootDomain, protocol }) => { | ||
await page.goto(`${protocol ?? 'http'}//bafkqablimvwgy3y.ipfs.${rootDomain}`, { waitUntil: 'networkidle' }) | ||
|
||
await expect(getNoServiceWorkerError(page)).toBeVisible() | ||
}) | ||
}) |
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.
Note that this doesn't test no service worker functionality in chrome, but we shouldn't need to...?