Skip to content

Commit

Permalink
- Refactored favicon functionality and types
Browse files Browse the repository at this point in the history
- Removed outgoing favicon API requests in E2E tests
  • Loading branch information
michaelschwobe committed Oct 25, 2023
1 parent 1ceb60d commit 352f09d
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 51 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ TagsForDays extends traditional bookmarking with advanced organization and searc
- TODO: Tag: popular widget (relation count)
- TODO: Tag: tag colors or other fields

- TODO: Bookmark: Reenable favicon fetching when more performant
- TODO: Bookmark: assess orderBy (title, date, etc.)
- TODO: Bookmark: export/selection (text, csv, json, html)
- TODO: Bookmark: watchtower route (dead links, redirects, etc.)
Expand Down
8 changes: 4 additions & 4 deletions app/components/bookmarks-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ import {
} from "~/components/ui/table";
import { cn } from "~/utils/misc";

export interface BookmarksWithFavicon {
export interface BookmarksWithFaviconSrc {
id: string;
title: string | null;
url: string;
favorite: boolean | null;
favicon: null;
faviconSrc: string | null;
}

const columnHelper = createColumnHelper<BookmarksWithFavicon>();
const columnHelper = createColumnHelper<BookmarksWithFaviconSrc>();

export const bookmarksTableColumns = [
columnHelper.display({
Expand Down Expand Up @@ -74,7 +74,7 @@ export const bookmarksTableColumns = [
cell: ({ row, getValue }) => (
<ButtonTitle
bookmarkId={row.original.id}
faviconSrc={row.original.favicon}
faviconSrc={row.original.faviconSrc}
title={getValue()}
/>
),
Expand Down
6 changes: 3 additions & 3 deletions app/components/latest-bookmarks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { H2 } from "~/components/ui/h2";
import { Icon } from "~/components/ui/icon";
import { LinkButton } from "~/components/ui/link-button";
import type { LatestBookmarksData } from "~/models/bookmark.server";
import type { BookmarksWithFaviconData } from "~/models/favicon.server";
import type { ItemsWithFaviconSrcData } from "~/models/favicon.server";
import { cn } from "~/utils/misc";
import { USER_LOGIN_ROUTE } from "~/utils/user";

Expand All @@ -19,7 +19,7 @@ export interface LatestBookmarksProps
/** Sets the `class` attribute. */
className?: string | undefined;
/** Sets the "found" content. **Required** */
data: BookmarksWithFaviconData<LatestBookmarksData>;
data: ItemsWithFaviconSrcData<LatestBookmarksData>;
}

export const LatestBookmarks = forwardRef<
Expand Down Expand Up @@ -47,7 +47,7 @@ export const LatestBookmarks = forwardRef<
className="max-w-[18rem] basis-1/3 justify-start overflow-hidden"
variant="ghost"
>
<Favicon src={bookmark.favicon} />{" "}
<Favicon src={bookmark.faviconSrc} />{" "}
<span className="truncate text-sm">
{bookmark.title ? (
<span>{bookmark.title}</span>
Expand Down
34 changes: 20 additions & 14 deletions app/models/favicon.server.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import { toFaviconServiceUrl } from "~/utils/bookmark";

type ArrayElementType<T> = T extends (infer E)[] ? E : T;

type Favicon = string | null;
type FaviconSrc = string | null;

type BookmarksWithUrlProp<T> = Array<ArrayElementType<T> & { url: string }>;
type ItemsWithUrlProp<T> = Array<ArrayElementType<T> & { url: string }>;

type BookmarksWithFaviconProp<T> = Array<
ArrayElementType<T> & { favicon: Favicon }
type ItemsWithFaviconSrcProp<T> = Array<
ArrayElementType<T> & { faviconSrc: FaviconSrc }
>;

export async function getFavicon(url: string): Promise<Favicon> {
export function toFaviconServiceUrl(url: string) {
const { hostname } = new URL(url);
const faviconServiceUrl = new URL("https://icons.duckduckgo.com/ip3");
faviconServiceUrl.pathname += `/${hostname}.ico`;
return faviconServiceUrl.href;
}

export async function getFavicon(url: string): Promise<FaviconSrc> {
try {
const faviconServiceUrl = toFaviconServiceUrl(url);
const response = await fetch(faviconServiceUrl);
Expand All @@ -23,16 +28,17 @@ export async function getFavicon(url: string): Promise<Favicon> {
}
}

export async function mapBookmarksWithFavicon<
T extends BookmarksWithUrlProp<T>,
>(bookmarks: T): Promise<BookmarksWithFaviconProp<T>> {
export async function mapWithFaviconSrc<T extends ItemsWithUrlProp<T>>(
bookmarks: T,
): Promise<ItemsWithFaviconSrcProp<T>> {
return Promise.all(
bookmarks.map(async (bookmark) => {
const favicon = await getFavicon(bookmark.url);
return { ...bookmark, favicon };
const faviconSrc = await getFavicon(bookmark.url);
return { ...bookmark, faviconSrc };
}),
);
}

export type BookmarksWithFaviconData<T extends BookmarksWithUrlProp<T>> =
Awaited<ReturnType<typeof mapBookmarksWithFavicon<T>>>;
export type ItemsWithFaviconSrcData<T extends ItemsWithUrlProp<T>> = Awaited<
ReturnType<typeof mapWithFaviconSrc<T>>
>;
9 changes: 4 additions & 5 deletions app/routes/_index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,18 @@ import { Main } from "~/components/main";
import { QuickBookmark } from "~/components/quick-bookmark";
import { QuickTag } from "~/components/quick-tag";
import { getLatestBookmarks } from "~/models/bookmark.server";
import { mapBookmarksWithFavicon } from "~/models/favicon.server";
import { mapWithFaviconSrc } from "~/models/favicon.server";
import { getLatestTags } from "~/models/tag.server";
import { generateSocialMeta } from "~/utils/meta";

export async function loader() {
const [latestBookmarks, latestTags] = await Promise.all([
const [latestBookmarksResults, latestTags] = await Promise.all([
getLatestBookmarks(),
getLatestTags({ take: 9 }),
]);
const latestBookmarksWithFavicon =
await mapBookmarksWithFavicon(latestBookmarks);
const latestBookmarks = await mapWithFaviconSrc(latestBookmarksResults);

return json({ latestBookmarks: latestBookmarksWithFavicon, latestTags });
return json({ latestBookmarks, latestTags });
}

export const meta: MetaFunction<typeof loader> = () => {
Expand Down
14 changes: 7 additions & 7 deletions app/routes/bookmarks.$bookmarkId._index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
favoriteBookmark,
getBookmark,
} from "~/models/bookmark.server";
import { mapBookmarksWithFavicon } from "~/models/favicon.server";
import { mapWithFaviconSrc } from "~/models/favicon.server";
import { requireUserId } from "~/utils/auth.server";
import { FavoriteBookmarkFormSchema } from "~/utils/bookmark-validation";
import { generateSocialMeta } from "~/utils/meta";
Expand All @@ -41,13 +41,13 @@ export async function loader({ params }: LoaderFunctionArgs) {
invariant(params["bookmarkId"], "bookmarkId not found");
const { bookmarkId: id } = params;

const bookmark = await getBookmark({ id });
invariantResponse(bookmark, "Not Found", { status: 404 });
const bookmarkResult = await getBookmark({ id });
invariantResponse(bookmarkResult, "Not Found", { status: 404 });

const [bookmarkWithFavicon] = await mapBookmarksWithFavicon([bookmark]);
invariant(bookmarkWithFavicon, "bookmarkWithFavicon not found");
const [bookmark] = await mapWithFaviconSrc([bookmarkResult]);
invariant(bookmark, "bookmark not found");

return json({ bookmark: bookmarkWithFavicon });
return json({ bookmark });
}

export async function action({ params, request }: ActionFunctionArgs) {
Expand Down Expand Up @@ -125,7 +125,7 @@ export default function BookmarkDetailPage() {
rel="noopener noreferrer"
className="justify-between overflow-hidden max-sm:w-full"
>
<Favicon src={loaderData.bookmark.favicon} />
<Favicon src={loaderData.bookmark.faviconSrc} />
<span className="truncate">{loaderData.bookmark.url}</span>
<Icon type="external-link" />
</LinkButton>{" "}
Expand Down
27 changes: 17 additions & 10 deletions app/routes/bookmarks._index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { H1 } from "~/components/ui/h1";
import { Icon } from "~/components/ui/icon";
import { LinkButton } from "~/components/ui/link-button";
import { getBookmarks } from "~/models/bookmark.server";
import { mapWithFaviconSrc } from "~/models/favicon.server";
import {
BOOKMARK_SEARCH_KEYS,
BOOKMARK_SEARCH_KEYS_LABEL_MAP,
Expand All @@ -28,11 +29,19 @@ export async function loader({ request }: LoaderFunctionArgs) {
const searchKey = parseBookmarkSearchKey(url.searchParams.get("searchKey"));
const searchValue = url.searchParams.get("searchValue");

const bookmarks = await getBookmarks({ searchKey, searchValue });
// const bookmarksWithFavicon = await mapBookmarksWithFavicon(bookmarks);
const bookmarksWithFavicon = bookmarks.map((b) => ({ ...b, favicon: null }));
const bookmarksResult = await getBookmarks({ searchKey, searchValue });
const bookmarksLength = bookmarksResult.length;
const bookmarks = await mapWithFaviconSrc(bookmarksResult);

return json({ bookmarks: bookmarksWithFavicon, searchKey, searchValue });
const hasBookmarks = bookmarksLength > 0;

return json({
bookmarks,
bookmarksLength,
hasBookmarks,
searchKey,
searchValue,
});
}

export const meta: MetaFunction<typeof loader> = ({ data }) => {
Expand All @@ -41,7 +50,7 @@ export const meta: MetaFunction<typeof loader> = ({ data }) => {
const description =
data?.searchKey && data?.searchValue
? `${formatItemsFoundByCount({
count: data?.bookmarks.length ?? 0,
count: data?.bookmarksLength ?? 0,
singular: "bookmark",
plural: "bookmarks",
})} within '${
Expand All @@ -59,14 +68,12 @@ export const meta: MetaFunction<typeof loader> = ({ data }) => {
export default function BookmarksIndexPage() {
const loaderData = useLoaderData<typeof loader>();

const bookmarksCount = loaderData.bookmarks.length;

return (
<Main>
<div className="mb-4 flex items-center gap-2">
<H1>
<Icon type="bookmarks" />
Bookmarks <Badge aria-hidden>{bookmarksCount}</Badge>
Bookmarks <Badge aria-hidden>{loaderData.bookmarksLength}</Badge>
</H1>
<LinkButton to={`${USER_LOGIN_ROUTE}?redirectTo=/bookmarks/import`}>
<Icon type="upload" />
Expand All @@ -92,7 +99,7 @@ export default function BookmarksIndexPage() {

<SearchHelp
className="mb-4"
count={bookmarksCount}
count={loaderData.bookmarksLength}
singular="bookmark"
plural="bookmarks"
>
Expand All @@ -117,7 +124,7 @@ export default function BookmarksIndexPage() {
</LinkButton>
</SearchHelp>

{bookmarksCount > 0 ? (
{loaderData.hasBookmarks ? (
<BookmarksTable
// TODO: remove ts-expect-error once this is fixed
// @ts-expect-error - node module bug https://github.com/TanStack/table/issues/5135
Expand Down
7 changes: 0 additions & 7 deletions app/utils/bookmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,3 @@ export function parseBookmarkSearchKey(
? (key as BookmarkSearchKey)
: null;
}

export function toFaviconServiceUrl(url: string) {
const { hostname } = new URL(url);
const faviconServiceUrl = new URL("https://icons.duckduckgo.com/ip3");
faviconServiceUrl.pathname += `/${hostname}.ico`;
return faviconServiceUrl.href;
}
6 changes: 6 additions & 0 deletions tests/e2e/_index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import { expect, login, logout, test } from "../utils/playwright-test-utils";

test.describe("Unauthenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
await page.goto("/");
});

Expand Down Expand Up @@ -77,6 +80,9 @@ test.describe("Unauthenticated", () => {

test.describe("Authenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
const { redirectTo } = await login({ page });
await page.waitForURL(redirectTo);
});
Expand Down
6 changes: 6 additions & 0 deletions tests/e2e/bookmarks.$bookmarkId._index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import { expect, login, logout, test } from "../utils/playwright-test-utils";

test.describe("Unauthenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
await page.goto("/bookmarks/bid2");
});

Expand Down Expand Up @@ -46,6 +49,9 @@ test.describe("Unauthenticated", () => {

test.describe("Authenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
const { redirectTo } = await login({
page,
to: "/bookmarks/bid2",
Expand Down
6 changes: 6 additions & 0 deletions tests/e2e/bookmarks._index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import { expect, login, logout, test } from "../utils/playwright-test-utils";

test.describe("Unauthenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
await page.goto("/bookmarks");
});

Expand Down Expand Up @@ -202,6 +205,9 @@ test.describe("Unauthenticated", () => {

test.describe("Authenticated", () => {
test.beforeEach(async ({ page }) => {
await page.route("https://icons.duckduckgo.com/ip3/**", async (route) => {
route.abort();
});
const { redirectTo } = await login({ page, to: "/bookmarks" });
await page.waitForURL(redirectTo);
});
Expand Down

0 comments on commit 352f09d

Please sign in to comment.