feature: CI/CD lint checks #156
Conversation
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Deploying openshockapp with
|
| Latest commit: |
85c30a1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eb4123a4.openshockapp.pages.dev |
| Branch Preview URL: | https://feature-cicd-checks.openshockapp.pages.dev |
…n permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds CI linting enforcement and updates SvelteKit navigation/linking patterns to satisfy stricter ESLint/Svelte lint rules (plus assorted lint cleanups across routes/components).
Changes:
- Add a GitHub Actions workflow to run
pnpm run linton PRs and key branches. - Tighten ESLint rules/ignores and apply widespread lint fixes (keyed
{#each}, unused imports, type tweaks). - Introduce URL helpers for base-prefixing and safe redirect handling; migrate some redirects from client
onMountto server+page.server.ts.
Reviewed changes
Copilot reviewed 80 out of 80 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| svelte.config.js | Removes unused branchName usage; leaves getGitBranch() behind with lint suppression. |
| src/routes/shares/public/[shareId=guid]/+page.svelte | Removes unused imports; still constructs ?redirect= from page.url.pathname. |
| src/routes/report/api-tokens/+page.svelte | Adds keyed {#each} for detected secrets. |
| src/routes/flashtool/FlashManager.ts | Converts single-line eslint disables to block-comment form. |
| src/routes/flashtool/FirmwareBoardSelector.svelte | Adds keyed {#each} for board list. |
| src/routes/Sidebar.svelte | Introduces unsafeResolve() usage for menu links; adjusts $app/paths imports. |
| src/routes/Header.svelte | Disables nav lint rule at file level; uses unsafeResolve() for navigation targets. |
| src/routes/Footer.svelte | Disables nav lint rule at file level (external link present). |
| src/routes/Breadcrumb.svelte | Adds keyed {#each} for breadcrumb list. |
| src/routes/+layout.svelte | Removes unused onMount import. |
| src/routes/+error.svelte | Tracks previous page; uses unsafeResolve() for “Go back” link; adjusts types/imports. |
| src/routes/(authenticated)/shockers/own/+page.svelte | Removes unused icon import. |
| src/routes/(authenticated)/shares/user/outgoing/user-share-item.svelte | Minor derived callback cleanup; keys share loop by share.id. |
| src/routes/(authenticated)/shares/user/outgoing/+page.svelte | Removes unused {:then fetched} binding. |
| src/routes/(authenticated)/shares/user/invites/outgoing-invite-item.svelte | Keys shocker loop by shocker.id. |
| src/routes/(authenticated)/shares/user/invites/incoming-invite-item.svelte | Keys shocker loop by shocker.id. |
| src/routes/(authenticated)/shares/user/invites/+page.svelte | Removes unused {:then fetched} bindings; simplifies keyed each. |
| src/routes/(authenticated)/shares/user/incoming/incoming-share-item.svelte | Keys loop by shocker.id. |
| src/routes/(authenticated)/shares/user/incoming/+page.svelte | Removes unused {:then fetched} binding. |
| src/routes/(authenticated)/shares/user/dialog-share-code-redeem.svelte | Removes unused icon imports; keys share loop by share.id. |
| src/routes/(authenticated)/shares/user/dialog-share-code-create.svelte | Removes unused icon import. |
| src/routes/(authenticated)/shares/user/+page.svelte | Removes client-side redirect page (deleted). |
| src/routes/(authenticated)/shares/user/+page.server.ts | Adds server-side redirect to outgoing shares. |
| src/routes/(authenticated)/shares/user/+layout.svelte | Tightens navigateTo typing and uses resolve() for navigation. |
| src/routes/(authenticated)/shares/public/dialog-publicshare-create.svelte | Removes unused date imports; keys expiration options loop. |
| src/routes/(authenticated)/shares/public/+page.svelte | Removes unused snippet/render imports. |
| src/routes/(authenticated)/settings/connections/+page.svelte | Adds keys to {#each} loops; minor {#each Array(4)} cleanup. |
| src/routes/(authenticated)/settings/api-tokens/new/+page.svelte | Keys permissions loop. |
| src/routes/(authenticated)/settings/api-tokens/dialog-token-edit.svelte | Keys permission category/perm loops. |
| src/routes/(authenticated)/settings/api-tokens/dialog-token-create.svelte | Keys expiration options and permission loops. |
| src/routes/(authenticated)/settings/account/DangerZone.svelte | Uses resolve('/') for navigation after deactivation. |
| src/routes/(authenticated)/settings/+page.svelte | Removes client-side redirect page (deleted). |
| src/routes/(authenticated)/settings/+page.server.ts | Adds server-side redirect to account settings. |
| src/routes/(authenticated)/hubs/dialog-hub-edit.svelte | Removes unused API import. |
| src/routes/(authenticated)/hubs/dialog-hub-delete.svelte | Keys shocker badge loop. |
| src/routes/(authenticated)/hubs/data-table-actions.svelte | Uses resolve() for update navigation; removes unused store import. |
| src/routes/(authenticated)/admin/webhooks/data-table-actions.svelte | Removes unused RoleType import. |
| src/routes/(authenticated)/admin/webhooks/+page.svelte | Removes unused CardContent import. |
| src/routes/(authenticated)/admin/users/dialog-user-edit.svelte | Keys roles loop. |
| src/routes/(authenticated)/admin/online-hubs/data-table-actions.svelte | Uses resolve() for “View User” navigation. |
| src/routes/(authenticated)/admin/config/+page.svelte | Removes unused CardContent import. |
| src/routes/(authenticated)/admin/+page.svelte | Removes client-side redirect page (deleted). |
| src/routes/(authenticated)/admin/+page.server.ts | Adds server-side redirect to online hubs. |
| src/routes/(authenticated)/+layout.svelte | Uses resolve('/login') for unauthenticated redirect. |
| src/routes/(anonymous)/signup/+page.svelte | Uses resolve('/login') after signup. |
| src/routes/(anonymous)/oauth/[provider]/create/+page.svelte | Uses resolve() for navigation; adds lint suppression for query-string navigation. |
| src/routes/(anonymous)/logout/+page.ts | Uses resolve('/') after logout. |
| src/routes/(anonymous)/login/+page.svelte | Uses new gotoQueryRedirectOrFallback() helper; makes SignalR init fire-and-forget. |
| src/routes/(anonymous)/forgot-password/+page.svelte | Uses resolve('/login') after request. |
| src/routes/(anonymous)/activate/+page.svelte | Uses resolve('/login') after activation. |
| src/routes/(anonymous)/+layout.svelte | Uses gotoQueryRedirectOrFallback() instead of direct query parsing. |
| src/lib/utils/url.ts | Adds unsafeResolve, redirect sanitization, and query-redirect navigation helper. |
| src/lib/types/ChildrenFunc.ts | Adds shared ChildrenFunc type. |
| src/lib/types/AnyComponent.ts | Adjusts eslint suppression for explicit any. |
| src/lib/typeguards/propGuards.ts | Replaces any with unknown in array item guard. |
| src/lib/stores/ConfirmDialogStore.ts | Adds eslint suppression for any-typed store. |
| src/lib/sharelink-signalr/index.svelte.ts | Adds lint suppression; removes unused callback arg. |
| src/lib/metadata.ts | Converts eslint disable to block-comment form. |
| src/lib/inputvalidation/usernameValidator.ts | Reorders/adds eslint disables; updates source link comment. |
| src/lib/components/utils/PauseToggle.svelte | Removes unused import. |
| src/lib/components/shares/user-selector.svelte | Types $props() destructuring. |
| src/lib/components/shares/permission-switch.svelte | Suppresses empty object type lint for Svelte component generic. |
| src/lib/components/metadata/OpenGraphTags.svelte | Keys locales loop. |
| src/lib/components/input/TextInput.svelte | Disables nav lint rule at file level; adds comment for external link. |
| src/lib/components/datetime-picker/time-picker-utils.ts | Adds braces around case '12hours' for linting. |
| src/lib/components/datetime-picker/date-time-picker.svelte | Simplifies inline callback. |
| src/lib/components/confirm-dialog/dialog-manager.svelte | Adds eslint suppression for any. |
| src/lib/components/LightSwitch.svelte | Keys scheme loop. |
| src/lib/components/Keyboard.svelte | Uses shared ChildrenFunc type. |
| src/lib/components/FirmwareChannelSelector.svelte | Keys firmware channel loop. |
| src/lib/components/ControlModules/impl/ShockerMenu.svelte | Uses resolve() for logs navigation. |
| src/lib/components/ControlModules/impl/ActionButtons.svelte | Keys buttons loop by type. |
| src/lib/components/ControlModules/SharedShockerControlModule.svelte | Removes unused icon import. |
| src/lib/components/ControlModules/PublicShareClassicControlModule.svelte | Removes unused icon imports. |
| src/lib/components/ControlModules/ControlListener.svelte | Removes unused type import. |
| src/lib/components/Container.svelte | Uses shared ChildrenFunc type. |
| src/lib/components/Code.svelte | Uses shared ChildrenFunc type. |
| src/lib/api/index.ts | Makes pathname const (lint cleanup). |
| eslint.config.js | Adds/changes lint rules and ignores additional directories. |
| .github/workflows/ci-lint.yml | Adds CI workflow to run lint. |
Comments suppressed due to low confidence (1)
svelte.config.js:34
getGitBranch()is now unused (andbranchNamehas been removed), but it’s kept with aneslint-disablecomment. Prefer removing the unused function entirely (or wire it back intokit.version/metadata) so this config stays minimal and avoids accumulating suppressed lint.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { CircleUser, LogIn, Undo2 } from '@lucide/svelte'; | ||
| import { goto } from '$app/navigation'; | ||
| import { resolve } from '$app/paths'; | ||
| import { page } from '$app/state'; | ||
| import { publicShockerSharesApi } from '$lib/api'; | ||
| import type { PublicShareResponse } from '$lib/api/internal/v1'; |
There was a problem hiding this comment.
This file builds ?redirect= using page.url.pathname + page.url.search, which includes the configured base path. With the new redirect helper (gotoQueryRedirectOrFallback → unsafeResolve), this can lead to double-base navigation under subpath deployments unless unsafeResolve accounts for already-based paths.
| <!-- I know this is deprecated buy resolve() is too strict to be used here... --> | ||
| <!-- eslint-disable-next-line svelte/no-navigation-without-resolve --> | ||
| <a href={unsafeResolve(menu.href)} {...props}> | ||
| <menu.Icon /> |
There was a problem hiding this comment.
menu.href is optional in the Menu type (menus with subItems don’t set href in this file). Calling unsafeResolve(menu.href) will produce base + undefined at runtime (and should be a TS error). Render an <a> only when menu.href is defined, or make unsafeResolve accept/handle undefined.
| import { asset, base, resolve } from '$app/paths'; | ||
| import { page } from '$app/state'; |
There was a problem hiding this comment.
base is imported from $app/paths but is not used anywhere in this file (it will now be flagged by unused-import lint rules). Remove the unused import.
| <!-- I know this is deprecated buy resolve() is too strict to be used here... --> | ||
| <!-- eslint-disable-next-line svelte/no-navigation-without-resolve --> | ||
| <a href={unsafeResolve(previousPage)}>Go back</a> |
There was a problem hiding this comment.
previousPage is derived from from.url.pathname, which already includes the app’s base when deployed under a subpath. Wrapping it in unsafeResolve() will double-prefix the base and can break the “Go back” link; use previousPage as-is (or make unsafeResolve a no-op when the pathname already starts with base).
| <script lang="ts"> | ||
| /* eslint-disable svelte/no-navigation-without-resolve */ | ||
|
|
||
| import { Input } from '$lib/components/ui/input'; | ||
| import type { AnyComponent } from '$lib/types/AnyComponent'; | ||
| import { GetValResColor, type ValidationResult } from '$lib/types/ValidationResult'; |
There was a problem hiding this comment.
File-level /* eslint-disable svelte/no-navigation-without-resolve */ disables the rule for the entire component, but the only unresolved navigation is the external link in the validation message. Prefer an inline disable on that <a> so internal navigation linting remains effective for future changes.
| import Breadcrumb from './Breadcrumb.svelte'; | ||
| import { base } from '$app/paths'; | ||
| import { unsafeResolve } from '$lib/utils/url'; | ||
|
|
There was a problem hiding this comment.
base is imported but unused in this file. With the new lint rules, this will be reported; please remove the unused import.
| <ScrollArea class="h-48 rounded-md border bg-gray-50 p-3"> | ||
| {#each secrets as secret} | ||
| {#each secrets as secret (secret)} | ||
| <p |
There was a problem hiding this comment.
The keyed {#each secrets as secret (secret)} will break if the pasted text contains duplicate tokens (keys must be unique). Since pasteFromClipboard() doesn’t dedupe, prefer a stable unique key (e.g., include the index or dedupe the array before rendering).
| export function load() { | ||
| redirect(307, resolve('/admin/online-hubs')); | ||
| } |
There was a problem hiding this comment.
redirect() from @sveltejs/kit must be thrown (or returned) from load. Calling it without throw will not trigger a redirect (see existing usage with throw redirect(...) in src/routes/usc/[code]/+server.ts).
| import { page } from '$app/state'; | ||
| import type { Pathname } from '$app/types'; | ||
|
|
||
| export function unsafeResolve(path: Pathname) { |
There was a problem hiding this comment.
unsafeResolve() unconditionally prefixes base, but many call sites (e.g. redirect query params built from page.url.pathname) already include the base path. This can produce broken URLs like ${base}${base}/... when deployed under a non-empty base path; consider making unsafeResolve detect already-based pathnames (or avoid using it on page.url.pathname).
| export function unsafeResolve(path: Pathname) { | |
| export function unsafeResolve(path: Pathname) { | |
| // Avoid double-prefixing when `path` already includes `base` | |
| if (!base || base === '/') { | |
| return path; | |
| } | |
| if (path === base || path.startsWith(base + '/')) { | |
| return path; | |
| } |
| import { afterNavigate } from '$app/navigation'; | ||
| import { asset, resolve } from '$app/paths'; | ||
| import { asset, base } from '$app/paths'; | ||
| import { page } from '$app/state'; | ||
| import type { Pathname } from '$app/types'; |
There was a problem hiding this comment.
base is imported but unused in this file. With the new lint rules, this will be reported; please remove the unused import.
1 participant
No description provided.