ldraney/pal-e-app

Archived

Fork 0

fix: resolve TypeScript and ESLint errors blocking CI #95

Merged

forgejo_admin merged 1 commit from 94-fix-lint-and-typescript-errors-blocking into main

2026-03-28 23:18:17 +00:00

forgejo_admin commented

2026-03-28 22:53:28 +00:00

Contributor

Summary

Fix all 8 TypeScript and ESLint errors blocking the CI check+lint stage. No Docker image has ever been built because the pipeline fails before reaching the build step, leaving the pod in ImagePullBackOff.

Changes

src/routes/boards/[slug]/+page.svelte -- add explicit TouchEvent and MouseEvent types to inline event handler parameters (2 TypeScript errors)
src/routes/+layout.svelte -- remove unused checkAuth import alias (1 ESLint error)
src/routes/graph/+page.svelte -- import SvelteSet from svelte/reactivity and use it for edgeSet and types; remove unused svelte-ignore comment (3 ESLint errors)
src/routes/notes/+page.svelte -- import SvelteSet and use it for types set (1 ESLint error)
src/routes/search/+page.svelte -- remove unused searchError variable, replace with console.error in catch block (1 ESLint error)

Test Plan

npm run check passes with 0 errors (1 pre-existing a11y warning)
npm run lint passes with 0 errors
No functional changes to application behavior

Review Checklist

No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #94
Parent issue: forgejo_admin/pal-e-platform #234
pal-e-docs -- project this work belongs to

## Summary Fix all 8 TypeScript and ESLint errors blocking the CI check+lint stage. No Docker image has ever been built because the pipeline fails before reaching the build step, leaving the pod in ImagePullBackOff. ## Changes - `src/routes/boards/[slug]/+page.svelte` -- add explicit `TouchEvent` and `MouseEvent` types to inline event handler parameters (2 TypeScript errors) - `src/routes/+layout.svelte` -- remove unused `checkAuth` import alias (1 ESLint error) - `src/routes/graph/+page.svelte` -- import `SvelteSet` from `svelte/reactivity` and use it for `edgeSet` and `types`; remove unused `svelte-ignore` comment (3 ESLint errors) - `src/routes/notes/+page.svelte` -- import `SvelteSet` and use it for `types` set (1 ESLint error) - `src/routes/search/+page.svelte` -- remove unused `searchError` variable, replace with `console.error` in catch block (1 ESLint error) ## Test Plan - [x] `npm run check` passes with 0 errors (1 pre-existing a11y warning) - [x] `npm run lint` passes with 0 errors - [ ] No functional changes to application behavior ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #94 - Parent issue: `forgejo_admin/pal-e-platform #234` - `pal-e-docs` -- project this work belongs to

forgejo_admin added 1 commit

2026-03-28 22:53:28 +00:00

fix: resolve TypeScript and ESLint errors blocking CI pipeline

ci/woodpecker/pr/woodpecker Pipeline failed

Details

e58d5bda4c

Type event parameters, replace Set with SvelteSet, remove unused
imports and variables to unblock the check+lint CI stage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 22:54:07 +00:00

Author

Contributor

QA Review

Scope: 5 files, +8/-10 lines. All changes target the 8 reported CI errors.

File-by-file

File	Fix	Verdict
`+layout.svelte`	Remove unused `checkAuth` import alias	OK -- `isAuthenticated` not used elsewhere in layout; auth state comes from `initKeycloak()` return value
`boards/[slug]/+page.svelte`	Type `e` params as `TouchEvent` and `MouseEvent`	OK -- correct DOM event types for `ontouchstart` and `onclick`
`graph/+page.svelte`	Import `SvelteSet`, use for `edgeSet` and `types`; remove unused `svelte-ignore`	OK -- `SvelteSet` is a drop-in for `Set` in these local contexts; the `a11y_click_events_have_key_events` ignore was unnecessary since `onkeydown` handler already exists
`notes/+page.svelte`	Import `SvelteSet`, use for `types`	OK -- same pattern as graph page
`search/+page.svelte`	Remove `searchError` variable, add `console.error` in catch	OK -- variable was never rendered in template; `console.error` preserves error visibility in devtools

Checks

No secrets or sensitive files
No unnecessary file changes
No functional behavior changes
Commit message follows repo convention (fix: ...)
Branch name follows convention (94-fix-lint-and-typescript-errors-blocking)

Nits

None.

VERDICT: APPROVED

## QA Review **Scope:** 5 files, +8/-10 lines. All changes target the 8 reported CI errors. ### File-by-file | File | Fix | Verdict | |------|-----|---------| | `+layout.svelte` | Remove unused `checkAuth` import alias | OK -- `isAuthenticated` not used elsewhere in layout; auth state comes from `initKeycloak()` return value | | `boards/[slug]/+page.svelte` | Type `e` params as `TouchEvent` and `MouseEvent` | OK -- correct DOM event types for `ontouchstart` and `onclick` | | `graph/+page.svelte` | Import `SvelteSet`, use for `edgeSet` and `types`; remove unused `svelte-ignore` | OK -- `SvelteSet` is a drop-in for `Set` in these local contexts; the `a11y_click_events_have_key_events` ignore was unnecessary since `onkeydown` handler already exists | | `notes/+page.svelte` | Import `SvelteSet`, use for `types` | OK -- same pattern as graph page | | `search/+page.svelte` | Remove `searchError` variable, add `console.error` in catch | OK -- variable was never rendered in template; `console.error` preserves error visibility in devtools | ### Checks - No secrets or sensitive files - No unnecessary file changes - No functional behavior changes - Commit message follows repo convention (`fix: ...`) - Branch name follows convention (`94-fix-lint-and-typescript-errors-blocking`) ### Nits None. --- **VERDICT: APPROVED**

forgejo_admin commented

2026-03-28 23:11:15 +00:00

Author

Contributor

PR #95 Review

DOMAIN REVIEW

Tech stack: SvelteKit / TypeScript (Svelte 5 runes mode)

File-by-file analysis (5 files, +8/-10 lines):

src/routes/+layout.svelte -- Removes isAuthenticated as checkAuth import. Confirmed: checkAuth is never referenced in the script or template. The layout uses a local authenticated variable set by initKeycloak(). Safe removal.
src/routes/boards/[slug]/+page.svelte -- Adds (e: TouchEvent) and (e: MouseEvent) type annotations to inline handlers. Verified against the handler signatures: onTouchStart(e: TouchEvent, item: BoardItem) at line 231 and onclick which receives MouseEvent per the DOM spec. Types are correct.
src/routes/graph/+page.svelte -- Imports SvelteSet from svelte/reactivity, changes edgeSet and types from new Set<string>() to new SvelteSet<string>(). Also removes  comment. The ignore removal is safe -- the <g> element already has an onkeydown handler (lines 347-349), so the a11y rule is satisfied.
src/routes/notes/+page.svelte -- Same SvelteSet pattern for the types local variable.
src/routes/search/+page.svelte -- Removes unused searchError state variable (was set but never read in the template), replaces the catch assignment with console.error('Search failed:', err). No user-visible behavior change -- the error was already invisible.

BLOCKERS

None.

All changes are pure lint/type cleanup with no functional behavior changes. No new functionality was added, so the "new functionality must have tests" criterion does not apply. No user input handling changed. No secrets. No auth path modifications.

NITS

SvelteSet for non-reactive locals: In graph/+page.svelte, edgeSet inside fetchAllNoteLinks() is a local dedup set that never touches the reactive template -- plain Set would suffice. Similarly types in both graph/+page.svelte and notes/+page.svelte are local variables used to build arrays that get spread into reactive state (noteTypes, availableTypes). SvelteSet adds minor overhead here. However, if this is what the Svelte 5 linter requires in .svelte files to pass svelte-check, it's acceptable as lint compliance. Worth revisiting if performance profiling ever flags the graph page.
Search error UX gap (pre-existing): The old searchError was never displayed in the template. The PR replaces it with console.error, which is equivalent. But the search page has no error state visible to the user at all -- a failed search silently produces no results. This is a pre-existing gap, not introduced by this PR, and out of scope for this fix.

SOP COMPLIANCE

Branch named after issue: 94-fix-lint-and-typescript-errors-blocking (issue #94)
PR body follows template: Summary, Changes, Test Plan, Related all present
Related references parent issue: forgejo_admin/pal-e-platform #234
No secrets committed
No unnecessary file changes (all 5 files directly address lint/TS errors)
Commit messages are descriptive

PROCESS OBSERVATIONS

This PR unblocks CI image builds for pal-e-docs-app. The 8 lint/TS errors have been preventing Docker image creation, which means the app has been in ImagePullBackOff. Merging this directly improves deployment frequency (DORA DF) by enabling the CI pipeline to produce artifacts for the first time.

The pre-existing search error UX gap (nit #2) would make a good follow-up ticket if not already tracked.

VERDICT: APPROVED

## PR #95 Review ### DOMAIN REVIEW **Tech stack**: SvelteKit / TypeScript (Svelte 5 runes mode) **File-by-file analysis (5 files, +8/-10 lines)**: 1. **`src/routes/+layout.svelte`** -- Removes `isAuthenticated as checkAuth` import. Confirmed: `checkAuth` is never referenced in the script or template. The layout uses a local `authenticated` variable set by `initKeycloak()`. Safe removal. 2. **`src/routes/boards/[slug]/+page.svelte`** -- Adds `(e: TouchEvent)` and `(e: MouseEvent)` type annotations to inline handlers. Verified against the handler signatures: `onTouchStart(e: TouchEvent, item: BoardItem)` at line 231 and `onclick` which receives `MouseEvent` per the DOM spec. Types are correct. 3. **`src/routes/graph/+page.svelte`** -- Imports `SvelteSet` from `svelte/reactivity`, changes `edgeSet` and `types` from `new Set<string>()` to `new SvelteSet<string>()`. Also removes `` comment. The ignore removal is safe -- the `<g>` element already has an `onkeydown` handler (lines 347-349), so the a11y rule is satisfied. 4. **`src/routes/notes/+page.svelte`** -- Same `SvelteSet` pattern for the `types` local variable. 5. **`src/routes/search/+page.svelte`** -- Removes unused `searchError` state variable (was set but never read in the template), replaces the catch assignment with `console.error('Search failed:', err)`. No user-visible behavior change -- the error was already invisible. ### BLOCKERS None. All changes are pure lint/type cleanup with no functional behavior changes. No new functionality was added, so the "new functionality must have tests" criterion does not apply. No user input handling changed. No secrets. No auth path modifications. ### NITS 1. **SvelteSet for non-reactive locals**: In `graph/+page.svelte`, `edgeSet` inside `fetchAllNoteLinks()` is a local dedup set that never touches the reactive template -- plain `Set` would suffice. Similarly `types` in both `graph/+page.svelte` and `notes/+page.svelte` are local variables used to build arrays that get spread into reactive state (`noteTypes`, `availableTypes`). `SvelteSet` adds minor overhead here. However, if this is what the Svelte 5 linter requires in `.svelte` files to pass `svelte-check`, it's acceptable as lint compliance. Worth revisiting if performance profiling ever flags the graph page. 2. **Search error UX gap (pre-existing)**: The old `searchError` was never displayed in the template. The PR replaces it with `console.error`, which is equivalent. But the search page has no error state visible to the user at all -- a failed search silently produces no results. This is a pre-existing gap, not introduced by this PR, and out of scope for this fix. ### SOP COMPLIANCE - [x] Branch named after issue: `94-fix-lint-and-typescript-errors-blocking` (issue #94) - [x] PR body follows template: Summary, Changes, Test Plan, Related all present - [x] Related references parent issue: `forgejo_admin/pal-e-platform #234` - [x] No secrets committed - [x] No unnecessary file changes (all 5 files directly address lint/TS errors) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS This PR unblocks CI image builds for pal-e-docs-app. The 8 lint/TS errors have been preventing Docker image creation, which means the app has been in ImagePullBackOff. Merging this directly improves deployment frequency (DORA DF) by enabling the CI pipeline to produce artifacts for the first time. The pre-existing search error UX gap (nit #2) would make a good follow-up ticket if not already tracked. ### VERDICT: APPROVED