From 4bb0e728411d849b5f0425b287a1835f388f36c5 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Fri, 1 Mar 2024 12:01:39 +0100 Subject: [PATCH] Remove hash routing Since we don't have or want any deployments on IPFS, and we don't have much use for hash routing else, we can clean up some code and remove some tests. --- .github/workflows/check-e2e.yml | 4 +- package.json | 3 - playwright.ipfs.config.ts | 14 -- src/App.svelte | 5 +- src/components/Markdown.svelte | 6 +- src/env.d.ts | 1 - src/lib/router.ts | 30 +--- src/lib/utils.ts | 4 +- tests/e2e/hashRouter.spec.ts | 165 ------------------ tests/e2e/project/patch.spec.ts | 2 +- .../e2e/{historyRouter.spec.ts => router.ts} | 0 11 files changed, 13 insertions(+), 221 deletions(-) delete mode 100644 playwright.ipfs.config.ts delete mode 100644 tests/e2e/hashRouter.spec.ts rename tests/e2e/{historyRouter.spec.ts => router.ts} (100%) diff --git a/.github/workflows/check-e2e.yml b/.github/workflows/check-e2e.yml index 4036f9cc3..dbddf7d9a 100644 --- a/.github/workflows/check-e2e.yml +++ b/.github/workflows/check-e2e.yml @@ -35,9 +35,7 @@ jobs: ./scripts/install-binaries; - name: Run Playwright tests - run: | - npm run test:e2e -- --project ${{ matrix.browser }}; - npm run test:e2e:ipfs -- --project ${{ matrix.browser }}; + run: npm run test:e2e -- --project ${{ matrix.browser }} - name: Upload artifacts uses: actions/upload-artifact@v4 diff --git a/package.json b/package.json index 7d58d72cf..0b27c059d 100644 --- a/package.json +++ b/package.json @@ -2,16 +2,13 @@ "version": "1.0.0", "scripts": { "start": "vite", - "start:ipfs": "VITE_HASH_ROUTING=1 vite --base ./", "serve": "vite preview", "build": "vite build && scripts/copy-katex-assets && scripts/install-twemoji-assets", - "build:ipfs": "VITE_HASH_ROUTING=1 vite build --base ./", "postinstall": "scripts/copy-katex-assets && scripts/install-twemoji-assets", "check": "scripts/check", "format": "npx prettier '**/*.@(ts|js|svelte|json|css|html|yml)' --write", "test:unit": "TZ='UTC' vitest run", "test:e2e": "TZ='UTC' playwright test", - "test:e2e:ipfs": "TZ='UTC' playwright test ./tests/e2e/hashRouter.spec.ts --config playwright.ipfs.config.ts", "test:httpd-api:unit": "TZ='UTC' vitest run --config httpd-client/vite.config.ts --reporter verbose" }, "type": "module", diff --git a/playwright.ipfs.config.ts b/playwright.ipfs.config.ts deleted file mode 100644 index 79a259c20..000000000 --- a/playwright.ipfs.config.ts +++ /dev/null @@ -1,14 +0,0 @@ -import type { PlaywrightTestConfig } from "@playwright/test"; - -import base from "./playwright.config.js"; - -const config: PlaywrightTestConfig = { - ...base, - testIgnore: undefined, - webServer: { - command: "npm run start:ipfs -- --strictPort --port 3001", - port: 3001, - }, -}; - -export default config; diff --git a/src/App.svelte b/src/App.svelte index 155a884d9..dd8f2c75b 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -32,10 +32,7 @@ void httpd.initialize().finally(() => void router.loadFromLocation()); if (!window.VITEST && !window.PLAYWRIGHT && import.meta.env.PROD) { - const plausible = Plausible({ - domain: "app.radicle.xyz", - hashMode: import.meta.env.VITE_HASH_ROUTING, - }); + const plausible = Plausible({ domain: "app.radicle.xyz" }); plausible.enableAutoPageviews(); } diff --git a/src/components/Markdown.svelte b/src/components/Markdown.svelte index 45e6e2b98..ee0fb293a 100644 --- a/src/components/Markdown.svelte +++ b/src/components/Markdown.svelte @@ -146,11 +146,7 @@ } // Make sure the source isn't a URL before trying to fetch it from the repo - if ( - imagePath && - !isUrl(imagePath) && - !imagePath.startsWith(`${router.base}twemoji`) - ) { + if (imagePath && !isUrl(imagePath) && !imagePath.startsWith("/twemoji")) { i.setAttribute("src", `${rawPath}/${canonicalize(imagePath, path)}`); } } diff --git a/src/env.d.ts b/src/env.d.ts index 5b01bee03..0acaa05a9 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -9,6 +9,5 @@ interface ImportMeta extends Readonly> { PROD: boolean; DEV: boolean; SSR: boolean; - VITE_HASH_ROUTING: boolean; }; } diff --git a/src/lib/router.ts b/src/lib/router.ts index 4885fe9b2..986d1698f 100644 --- a/src/lib/router.ts +++ b/src/lib/router.ts @@ -34,8 +34,6 @@ export function useDefaultNavigation(event: MouseEvent) { ); } -export const base = import.meta.env.VITE_HASH_ROUTING ? "./" : "/"; - export async function loadFromLocation(): Promise { await navigateToUrl("replace", new URL(window.location.href)); } @@ -44,28 +42,18 @@ export async function navigateToUrl( action: "push" | "replace", url: URL, ): Promise { - let { pathname, hash } = url; + const { pathname, hash } = url; if (url.origin !== window.origin) { throw new Error("Cannot navigate to other origin"); } - if (import.meta.env.VITE_HASH_ROUTING) { - if (pathname === "/" && hash && !hash.startsWith("#/")) { - // We land here if the user clicked an link with only a hash reference. - // Instead of going to the root page we stop routing here and have the - // browser take care of things. - return; - } - [pathname, hash] = hash.substring(1).split("#"); - } else { - if ( - currentUrl && - currentUrl.pathname === pathname && - currentUrl.search === url.search - ) { - return; - } + if ( + currentUrl && + currentUrl.pathname === pathname && + currentUrl.search === url.search + ) { + return; } const relativeUrl = pathname + url.search + (hash || ""); @@ -91,9 +79,7 @@ async function navigate( newRoute: Route, ): Promise { isLoading.set(true); - const path = import.meta.env.VITE_HASH_ROUTING - ? "#" + routeToPath(newRoute) - : routeToPath(newRoute); + const path = routeToPath(newRoute); if (action === "push") { window.history.pushState(newRoute, "", path); diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 340f8c9d7..6d556481c 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -4,8 +4,6 @@ import md5 from "md5"; import bs58 from "bs58"; import twemojiModule from "twemoji"; -import { base } from "@app/lib/router"; - export async function toClipboard(text: string): Promise { await navigator.clipboard.writeText(text); } @@ -278,7 +276,7 @@ export function twemoji( ? false : "".concat(options.base, options.size, "/", icon, options.ext); }, - base, + base: "/", folder: "twemoji", ext: ".svg", className: `txt-emoji`, diff --git a/tests/e2e/hashRouter.spec.ts b/tests/e2e/hashRouter.spec.ts deleted file mode 100644 index df7c42562..000000000 --- a/tests/e2e/hashRouter.spec.ts +++ /dev/null @@ -1,165 +0,0 @@ -import { - aliceMainHead, - aliceRemote, - appConfigWithFixture, - expect, - sourceBrowsingUrl, - test, -} from "@tests/support/fixtures.js"; -import { - expectBackAndForwardNavigationWorks, - expectUrlPersistsReload, -} from "@tests/support/router.js"; - -test("navigate between landing and project page", async ({ page }) => { - await page.addInitScript(appConfigWithFixture); - - await page.goto("/#/"); - await expect(page).toHaveURL("/#/"); - - await page.getByText("source-browsing").click(); - await expect(page).toHaveURL(`/#${sourceBrowsingUrl}`); - - await expectBackAndForwardNavigationWorks("/#/", page); - await expectUrlPersistsReload(page); -}); - -test("navigation between node and project pages", async ({ page }) => { - await page.goto("/#/nodes/radicle.local"); - - const project = page - .locator(".project-card", { hasText: "source-browsing" }) - .nth(0); - await project.click(); - await expect(page).toHaveURL(`/#${sourceBrowsingUrl}`); - - await expectBackAndForwardNavigationWorks("/#/nodes/radicle.local", page); - await expectUrlPersistsReload(page); - - await page.getByRole("link", { name: "Local Node" }).click(); - await expect(page).toHaveURL("/#/nodes/127.0.0.1"); -}); - -test.describe("project page navigation", () => { - test("navigation between commit history and single commit", async ({ - page, - }) => { - const projectHistoryURL = `/#${sourceBrowsingUrl}/history/${aliceMainHead}`; - await page.goto(projectHistoryURL); - - await page.getByText("Add README.md").click(); - await expect(page).toHaveURL( - `/#${sourceBrowsingUrl}/commits/${aliceMainHead}`, - ); - - await expectBackAndForwardNavigationWorks(projectHistoryURL, page); - await expectUrlPersistsReload(page); - }); - - test("navigate between tree and commit history", async ({ page }) => { - const projectTreeURL = `/#${sourceBrowsingUrl}/tree/${aliceMainHead}`; - - await page.goto(projectTreeURL); - await page - .getByRole("progressbar", { name: "Page loading" }) - .waitFor({ state: "hidden" }); - await expect(page).toHaveURL(projectTreeURL); - - await page.getByRole("link", { name: "Commits 6" }).click(); - await expect(page).toHaveURL( - `/#${sourceBrowsingUrl}/history/${aliceMainHead}`, - ); - - await expectBackAndForwardNavigationWorks(projectTreeURL, page); - await expectUrlPersistsReload(page); - }); - - test("navigate between tree and commit history while a file is selected", async ({ - page, - }) => { - const projectTreeURL = `/#${sourceBrowsingUrl}`; - - await page.goto(projectTreeURL); - await page - .getByRole("progressbar", { name: "Page loading" }) - .waitFor({ state: "hidden" }); - await expect(page).toHaveURL(projectTreeURL); - - await page.getByText(".hidden").click(); - await expect(page).toHaveURL(`${projectTreeURL}/tree/.hidden`); - - await page.getByRole("link", { name: "Commits 6" }).click(); - await expect(page).toHaveURL(`${projectTreeURL}/history`); - }); - - test("navigate project paths", async ({ page }) => { - const projectTreeURL = `/#${sourceBrowsingUrl}/tree/${aliceMainHead}`; - - await page.goto(projectTreeURL); - await expect(page).toHaveURL(projectTreeURL); - - await page.getByText(".hidden").click(); - await expect(page).toHaveURL(`${projectTreeURL}/.hidden`); - - await page.getByText("bin").click(); - await page.getByText("true").click(); - await expect(page).toHaveURL(`${projectTreeURL}/bin/true`); - - await expectBackAndForwardNavigationWorks( - `${projectTreeURL}/.hidden`, - page, - ); - await expectUrlPersistsReload(page); - }); - - test("navigate project paths with an explicitly selected peer", async ({ - page, - }) => { - // If a branch isn't explicitly specified, the code assumes the project - // default branch is selected. We omit showing the default branch in the URL. - - const projectTreeURL = `/#${sourceBrowsingUrl}/remotes/${aliceRemote.substring( - 8, - )}`; - - await page.goto(projectTreeURL); - await expect(page).toHaveURL(projectTreeURL); - - await page.getByText(".hidden").click(); - await expect(page).toHaveURL(`${projectTreeURL}/tree/.hidden`); - - await page.getByText("bin").click(); - await page.getByText("true").click(); - await expect(page).toHaveURL(`${projectTreeURL}/tree/bin/true`); - - await expectBackAndForwardNavigationWorks( - `${projectTreeURL}/tree/.hidden`, - page, - ); - await expectUrlPersistsReload(page); - }); - - test("navigate project paths with an explicitly selected peer and branch", async ({ - page, - }) => { - const projectTreeURL = `/#${sourceBrowsingUrl}/remotes/${aliceRemote.substring( - 8, - )}/tree/main`; - - await page.goto(projectTreeURL); - await expect(page).toHaveURL(projectTreeURL); - - await page.getByText(".hidden").click(); - await expect(page).toHaveURL(`${projectTreeURL}/.hidden`); - - await page.getByText("bin").click(); - await page.getByText("true").click(); - await expect(page).toHaveURL(`${projectTreeURL}/bin/true`); - - await expectBackAndForwardNavigationWorks( - `${projectTreeURL}/.hidden`, - page, - ); - await expectUrlPersistsReload(page); - }); -}); diff --git a/tests/e2e/project/patch.spec.ts b/tests/e2e/project/patch.spec.ts index 1b9495206..5b3e7851f 100644 --- a/tests/e2e/project/patch.spec.ts +++ b/tests/e2e/project/patch.spec.ts @@ -101,7 +101,7 @@ test("navigate through revision diffs", async ({ page }) => { .getByRole("button") .click(); await expect( - page.getByRole("link", { name: "Compare 88b7fd9..9e4fea" }), + page.getByRole("link", { name: "Compare 88b7fd..9e4fea" }), ).toBeVisible(); await page.goBack(); } diff --git a/tests/e2e/historyRouter.spec.ts b/tests/e2e/router.ts similarity index 100% rename from tests/e2e/historyRouter.spec.ts rename to tests/e2e/router.ts