feat: add in-app changelog notifications#1450
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds an in-app changelog notification system: a dismissible card pinned to the bottom of the left sidebar that fetches release notes from the emdash.sh changelog API (with HTML-to-markdown fallback), persists dismissal state per-version in app settings, and opens a full-screen modal for the rendered changelog content. Key changes:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/renderer/hooks/useChangelogNotification.ts | New hook that drives the notification card; contains a logic bug where dismissing the available-version notification immediately re-surfaces the installed-version notification (since the two versions are tracked independently in dismissedVersions). |
| src/main/services/ChangelogService.ts | New service fetching changelog data from JSON APIs with HTML fallback; contains previously-flagged issues (no request timeout, direct-entry check before collection extraction); core parsing logic is solid with good fallback handling. |
| src/main/settings.ts | Adds changelog.dismissedVersions to the settings schema with normalization; normalization strips leading v but does not validate semver format (unlike normalizeChangelogVersion), allowing arbitrary strings to persist. |
| src/shared/changelog.ts | New shared types and utilities; normalizeChangelogVersion correctly validates semver format, compareChangelogVersions handles pre-release ordering per semver semantics. No issues found. |
| src/renderer/components/ChangelogModal.tsx | New modal for displaying changelog content; stripLeadingReleaseHeadings can produce false-positive stripping via its concatenated set member; integrates cleanly with the existing ModalRenderer/Dialog wrapper for close handling. |
| src/renderer/components/sidebar/ChangelogNotificationCard.tsx | New compact dismissible card component; accessible with aria-label on the dismiss button, clean separation between open and dismiss actions. No issues found. |
| src/renderer/components/sidebar/LeftSidebar.tsx | Integrates the changelog card below the scrollable project list using a flex layout restructure; the layout change (min-h-0 overflow-hidden) is necessary for the pinned bottom card and looks correct. |
| src/renderer/contexts/ModalProvider.tsx | Registers ChangelogModalOverlay in the modal registry; the existing ModalRenderer Dialog wrapper correctly handles close-on-X-button for all registered modals including the new one. No issues found. |
| src/main/ipc/changelogIpc.ts | Thin RPC controller wrapping changelogService.getLatestEntry; follows the established pattern correctly. No issues found. |
| src/main/ipc/index.ts | Registers the new changelogController in the RPC router; minimal change, no issues. |
| src/renderer/components/UpdateModal.tsx | Migrates the changelog link from EMDASH_RELEASES_URL to the shared EMDASH_CHANGELOG_URL constant; straightforward change, no issues found. |
| src/test/main/ChangelogService.test.ts | New tests for normalizeChangelogVersion, compareChangelogVersions, and parseChangelogHtml; covers version normalization, semver comparison, and HTML parsing. No issues found. |
| src/test/main/settings.test.ts | Adds tests for changelog.dismissedVersions normalization (deduplication, trimming, default empty list); no missing test cases for the validation gap flagged in settings.ts. |
Sequence Diagram
sequenceDiagram
participant R as Renderer (useChangelogNotification)
participant IPC as changelogIpc (main)
participant CS as ChangelogService
participant API as emdash.sh API
participant S as AppSettings
R->>R: read installedVersion, availableVersion
R->>R: selectVersion() → notificationVersion
R->>IPC: rpc.changelog.getLatestEntry({ version })
IPC->>CS: getLatestEntry(version)
CS->>API: GET /api/changelog?version=X (JSON)
alt JSON success
API-->>CS: ChangelogEntry JSON
CS-->>IPC: ChangelogEntry
else JSON fails
CS->>API: GET /changelog (HTML fallback)
API-->>CS: HTML page
CS->>CS: parseChangelogHtml()
CS-->>IPC: ChangelogEntry
end
IPC-->>R: ChangelogEntry (or null)
R->>R: show ChangelogNotificationCard
note over R: User clicks Dismiss
R->>R: dismiss() – adds notificationVersion to dismissedVersions
R->>S: updateSettings({ changelog: { dismissedVersions } })
S-->>R: settings invalidated & refetched
R->>R: selectVersion() re-evaluated → may show installedVersion card again ⚠️
Last reviewed commit: 442c41d
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function extractCandidatesFromPayload(payload: unknown): ChangelogCandidate[] { | ||
| if (!payload || typeof payload !== 'object') return []; | ||
|
|
||
| if (Array.isArray(payload)) { | ||
| return payload.filter((item): item is ChangelogCandidate => !!item && typeof item === 'object'); | ||
| } | ||
|
|
||
| const record = payload as Record<string, unknown>; | ||
| const directCandidate = normalizeEntry(record as ChangelogCandidate); | ||
| if (directCandidate) return [record as ChangelogCandidate]; | ||
|
|
||
| const collections = ['entry', 'release', 'item', 'entries', 'items', 'releases', 'data']; | ||
| for (const key of collections) { | ||
| const value = record[key]; | ||
| if (Array.isArray(value)) { | ||
| return value.filter((item): item is ChangelogCandidate => !!item && typeof item === 'object'); | ||
| } | ||
| if (value && typeof value === 'object') { | ||
| return [value as ChangelogCandidate]; | ||
| } | ||
| } | ||
|
|
||
| return []; | ||
| } |
There was a problem hiding this comment.
Direct-entry check short-circuits collection extraction
normalizeEntry at line 207 succeeds for any object that carries a valid version field — even if that same object also contains a collection key (e.g. releases, items, or data). Because the early return on line 208 fires before the collections loop is reached, a response shaped like:
{ "version": "1.0.0", "releases": [{ … }, { … }] }would be returned as a single, content-less candidate instead of the full list of releases.
Consider moving the directCandidate check to the end as a last-resort fallback, after the collections loop has already had a chance to find a richer payload:
function extractCandidatesFromPayload(payload: unknown): ChangelogCandidate[] {
if (!payload || typeof payload !== 'object') return [];
if (Array.isArray(payload)) {
return payload.filter((item): item is ChangelogCandidate => !!item && typeof item === 'object');
}
const record = payload as Record<string, unknown>;
const collections = ['entry', 'release', 'item', 'entries', 'items', 'releases', 'data'];
for (const key of collections) {
const value = record[key];
if (Array.isArray(value)) {
return value.filter((item): item is ChangelogCandidate => !!item && typeof item === 'object');
}
if (value && typeof value === 'object') {
return [value as ChangelogCandidate];
}
}
// Last resort: treat the record itself as a single candidate
const directCandidate = normalizeEntry(record as ChangelogCandidate);
if (directCandidate) return [record as ChangelogCandidate];
return [];
}| async function fetchJson(url: string): Promise<unknown | null> { | ||
| const response = await fetch(url, { | ||
| headers: { | ||
| Accept: 'application/json, text/plain;q=0.9, */*;q=0.8', | ||
| 'Cache-Control': 'no-cache', | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.ok) return null; | ||
|
|
||
| const contentType = response.headers.get('content-type') ?? ''; | ||
| if (!contentType.includes('json')) return null; | ||
|
|
||
| return response.json(); | ||
| } |
There was a problem hiding this comment.
No timeout on network requests
Both fetchJson and the HTML fallback fetch call in getLatestEntry have no timeout. In an Electron app where the machine is offline or the server is unresponsive, these requests will hang until the OS TCP timeout fires (potentially minutes), keeping the changelog query in a perpetual loading state.
Consider wrapping each fetch with an AbortSignal.timeout:
async function fetchJson(url: string): Promise<unknown | null> {
const response = await fetch(url, {
signal: AbortSignal.timeout(10_000),
headers: {
Accept: 'application/json, text/plain;q=0.9, */*;q=0.8',
'Cache-Control': 'no-cache',
},
});
// ...
}And similarly for the HTML fallback fetch in getLatestEntry (line 336).
| const dismissedVersions = settings?.changelog?.dismissedVersions ?? []; | ||
| const installedVersion = normalizeChangelogVersion(appVersion); | ||
| const notificationVersion = useMemo( | ||
| () => selectVersion(installedVersion, availableVersion, dismissedVersions), | ||
| [installedVersion, availableVersion, dismissedVersions] | ||
| ); |
There was a problem hiding this comment.
Unstable dismissedVersions reference breaks useMemo memoization
When settings has not yet loaded (or settings.changelog is absent), settings?.changelog?.dismissedVersions ?? [] evaluates the ?? fallback path and allocates a new [] on every render. Because dismissedVersions is a dependency of the notificationVersion useMemo, the memo will recompute on every render during that window.
Stabilise the reference with useMemo:
const dismissedVersions = useMemo(
() => settings?.changelog?.dismissedVersions ?? [],
[settings?.changelog?.dismissedVersions]
);Or, if you prefer, derive it inline with a separate stable default:
const dismissedVersions = settings?.changelog?.dismissedVersions ?? EMPTY_ARRAY;
// const EMPTY_ARRAY: string[] = []; // module-level constant
| const dismiss = useCallback(() => { | ||
| if (!notificationVersion) return; | ||
|
|
||
| const nextDismissedVersions = [...new Set([...dismissedVersions, notificationVersion])]; | ||
| updateSettings({ | ||
| changelog: { | ||
| dismissedVersions: nextDismissedVersions, | ||
| }, | ||
| }); | ||
| }, [dismissedVersions, notificationVersion, updateSettings]); |
There was a problem hiding this comment.
Dismissing the available-version notification re-shows the installed-version notification
dismiss only adds notificationVersion to dismissedVersions. When the user is shown the available version's changelog (e.g. "1.3.0") and taps Dismiss, only "1.3.0" is marked dismissed. On the very next render selectVersion evaluates:
visibleInstalled = "1.2.3" // not in dismissedVersions → still visible
visibleAvailable = null // just dismissed
→ returns "1.2.3" // card immediately reappears for the older version
This is especially likely to affect early adopters of this feature whose dismissedVersions list was previously empty: they see the availableVersion card, dismiss it, and instantly get a second card for the installedVersion.
The simplest fix is to dismiss every version that is currently visible (both installed and available) when the user clicks the dismiss button:
const dismiss = useCallback(() => {
if (!notificationVersion) return;
const allToDismiss = [notificationVersion, installedVersion, availableVersion].filter(
(v): v is string => v !== null
);
const nextDismissedVersions = [...new Set([...dismissedVersions, ...allToDismiss])];
updateSettings({ changelog: { dismissedVersions: nextDismissedVersions } });
}, [availableVersion, dismissedVersions, installedVersion, notificationVersion, updateSettings]);Alternatively selectVersion can treat a version as dismissed whenever any version greater-than-or-equal-to it is already in dismissedVersions, but the explicit dismissal approach above is simpler and easier to reason about.
|
|
||
| const rawDismissedVersions = (input as any)?.changelog?.dismissedVersions; | ||
| out.changelog = { | ||
| dismissedVersions: Array.isArray(rawDismissedVersions) | ||
| ? [ | ||
| ...new Set( | ||
| rawDismissedVersions | ||
| .filter((value: unknown): value is string => typeof value === 'string') | ||
| .map((value) => value.trim().replace(/^v/i, '')) | ||
| .filter(Boolean) | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Version strings not validated before persisting
The normalization here trims and strips the leading v, but does not validate that each entry is actually a well-formed version string. normalizeChangelogVersion in shared/changelog.ts already contains the right regex for this. Persisting strings like "latest", "beta", or " " (the last is filtered by .filter(Boolean)) will silently accumulate in the file.
Because dismissedVersions.includes(installedVersion) compares against an already-normalized semver string, junk entries won't cause a functional bug today, but they clutter the settings file and could confuse future logic that iterates the list.
Consider reusing normalizeChangelogVersion and filtering out nulls:
import { normalizeChangelogVersion } from '@shared/changelog';
// inside normalizeSettings:
const rawDismissedVersions = (input as any)?.changelog?.dismissedVersions;
out.changelog = {
dismissedVersions: Array.isArray(rawDismissedVersions)
? [
...new Set(
rawDismissedVersions
.map((v: unknown) =>
typeof v === 'string' ? normalizeChangelogVersion(v) : null
)
.filter((v): v is string => v !== null)
),
]
: [],
};| function stripLeadingReleaseHeadings(content: string, entry: ChangelogEntry): string { | ||
| const lines = content.split('\n'); | ||
| const normalizedVersion = normalizeLeadLine(`v${entry.version}`); | ||
| const normalizedTitle = normalizeLeadLine(entry.title); | ||
| const redundantLeads = new Set([ | ||
| normalizedVersion, | ||
| normalizeLeadLine(entry.version), | ||
| normalizeLeadLine("What's Changed"), | ||
| normalizeLeadLine(`v${entry.version} What's Changed`), | ||
| normalizeLeadLine(`${entry.version} What's Changed`), | ||
| normalizedTitle + normalizeLeadLine("What's Changed"), | ||
| ]); | ||
|
|
||
| while (lines.length > 0) { | ||
| const line = lines[0].trim(); | ||
| if (!line) { | ||
| lines.shift(); | ||
| continue; | ||
| } | ||
|
|
||
| const normalizedLine = normalizeLeadLine(line); | ||
| if (redundantLeads.has(normalizedLine) || normalizedLine === normalizedTitle) { | ||
| lines.shift(); | ||
| continue; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
|
|
||
| return lines.join('\n').trim(); | ||
| } |
There was a problem hiding this comment.
normalizeLeadLine removes all non-alphanumeric characters before set membership check
normalizeLeadLine strips every character except [a-zA-Z0-9.], so two structurally different headings can accidentally normalize to the same string. The most notable case is the concatenated set member:
normalizedTitle + normalizeLeadLine("What's Changed")
// → e.g. "changelognotifications" + "whatschanged"
// = "changelognotificationswhatschanged"A content heading literally named "ChangelogNotificationsWhatsChanged" would be incorrectly stripped. This is unlikely in practice, but the intent is clearer and safer if the check includes a separator:
redundantLeads.add(normalizedTitle + ' ' + normalizeLeadLine("What's Changed"));
// → "changelognotifications whatschanged" — won't collide with word-run titles(Or use a canonical separator that normalizeLeadLine always removes, e.g. a digit, to keep a single-token key space.)
1 participant
Summary
Changes
src/shared/changelog.ts— Shared types (ChangelogEntry), version normalization, and semver comparison utilitiessrc/main/services/ChangelogService.ts— Service that fetches changelog entries from JSON API endpoints with HTML parsing fallbacksrc/main/ipc/changelogIpc.ts— RPC controller exposinggetLatestEntryto the renderersrc/renderer/hooks/useChangelogNotification.ts— Hook that determines which version to show based on installed/available versions and dismissed listsrc/renderer/components/ChangelogModal.tsx— Modal displaying full changelog content with markdown renderingsrc/renderer/components/sidebar/ChangelogNotificationCard.tsx— Compact dismissible card shown at the sidebar bottomsrc/renderer/components/sidebar/LeftSidebar.tsx— Integrates the notification card; adjusts sidebar layout so the card pins below the scrollable project listsrc/main/settings.ts— Addschangelog.dismissedVersionsto settings schema with normalizationsrc/renderer/components/UpdateModal.tsx— Points the changelog link to the sharedEMDASH_CHANGELOG_URLconstantTest plan
pnpm run type-check,pnpm run lint, andpnpm exec vitest runall pass