-
Notifications
You must be signed in to change notification settings - Fork 1
β‘ Bolt: Optimize MediaGalleryView sorting and rendering #500
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## 2025-04-29 - O(n log n) overhead in localeCompare for ISO dates | ||
| **Learning:** `localeCompare` is heavily used across the codebase for sorting simple ISO timestamp strings (e.g., `createdAt`), which incurs a significant and unnecessary O(n log n) performance hit compared to raw string comparison. | ||
| **Action:** When sorting standard ISO 8601 strings, always replace `localeCompare` with raw comparison (`b > a ? 1 : b < a ? -1 : 0`). Additionally, ensure derived filtered lists in React are wrapped in `useMemo` to avoid redundant O(n) filtering on re-renders. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| * APIs (getDatabaseTables, executeDatabaseQuery). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCallback, useEffect, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useCallback, useEffect, useMemo, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { client, type QueryResult } from "../api-client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type MediaType = "all" | "image" | "video" | "audio"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -177,11 +177,16 @@ export function MediaGalleryView() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Sort by date descending | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // β‘ Bolt: Use raw string comparison instead of localeCompare for faster timestamp sorting | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| allMedia.sort((a, b) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!a.createdAt && !b.createdAt) return 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!a.createdAt) return 1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!b.createdAt) return -1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return b.createdAt.localeCompare(a.createdAt); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return b.createdAt > a.createdAt | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : b.createdAt < a.createdAt | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? -1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setMedia(allMedia); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -197,16 +202,20 @@ export function MediaGalleryView() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| loadMedia(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [loadMedia]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filtered = media.filter((m) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (filter !== "all" && m.type !== filter) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| search && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !m.filename.toLowerCase().includes(search.toLowerCase()) && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !m.url.toLowerCase().includes(search.toLowerCase()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // β‘ Bolt: Memoize the derived filtered list and hoist lowercase conversion to prevent O(n) reallocation and redundant conversions on re-renders | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filtered = useMemo(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const searchLower = search?.toLowerCase() ?? ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return media.filter((m) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (filter !== "all" && m.type !== filter) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchLower && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !m.filename.toLowerCase().includes(searchLower) && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| !m.url.toLowerCase().includes(searchLower) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [media, filter, search]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+206
to
+218
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. The
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Potentially Incorrect Date Sorting
The sorting logic for
allMediauses raw string comparison on thecreatedAtfield:This approach assumes that all
createdAtvalues are in a consistent, lexicographically sortable format (such as ISO 8601). If anycreatedAtvalues are in a different format or are malformed, the sort order may be incorrect, leading to unexpected results in the media gallery.Recommended Solution:
createdAtvalues are normalized to a consistent format (preferably ISO 8601) before sorting.Date.parse()ornew Date()for comparison: