Skip to content

chore(types): resolve all svelte-check errors and enforce svelte-check in CI#1243

Merged
chhoumann merged 1 commit into
masterfrom
fix/svelte-check-types
May 29, 2026
Merged

chore(types): resolve all svelte-check errors and enforce svelte-check in CI#1243
chhoumann merged 1 commit into
masterfrom
fix/svelte-check-types

Conversation

@chhoumann
Copy link
Copy Markdown
Owner

@chhoumann chhoumann commented May 29, 2026

Resolves the 40 svelte-check errors (17 files) and wires svelte-check into CI. Every fix is type-only / behavior-preserving — no Svelte version or bundling change (we share the Obsidian runtime with other plugins).

Fixes by category

  • dnd drag handles (14)startDrag was mis-typed (e: CustomEvent<DndEvent>) but is bound to on:mousedown/on:touchstart; retyped to (e: MouseEvent | TouchEvent) in the 7 command components + CommandList (kept the parent's DndEvent import — still used by consider/finalize; dropped the unused child imports).
  • Command subclasses (11)declare on inherited name/id/type (TS2612; emission-free under ES2020, and prevents field-clobbering if the target is ever raised).
  • CommandList narrowing (8)ICommand → specific types via <script> cast helpers passed one-way (command={asWait(command)}); children report edits via on:updateCommand events (verified none reassign command), so behavior is unchanged. ConstructorParameters<...> for the settings-modal arg.
  • choiceList (4) — broke the ChoiceList ↔ MultiChoiceListItem recursive type cycle with as any cross-imports; imported ChoiceType from its canonical source; typed reorder handler; removed the dead type="main" prop.
  • choiceSuggester (1) — initialize choiceExecutor in the constructor body (after super()) so plugin is defined (TS2729).
  • GlobalVariables (2) — dropped a dead id field never present on GV (confirmed unused; {#each} is index-based).

CI

Adds a check script (svelte-check --threshold error) and runs it in the Build + Lint job, so Svelte component type regressions are caught going forward.

Verification

  • svelte-check 0 errors / 0 warnings (was 40); build-with-lint clean; 1126 tests pass.
  • The provider/endpoint detection and other logic untouched.
  • Dev-vault smoke test: plugin reloads with no errors; the choice suggester (exercises the choiceSuggester init reorder) opens with no errors; settings render with no errors.

Investigated + cross-checked via a multi-agent workflow (each fix got a runtime-safety assessment); the agent that consolidated the plan empirically drove it to 0 svelte-check + 0 tsc errors.
Note: I couldn't fully drive the macro-editor modal (drag-reorder/edit a command) headlessly — that change is event-driven and compiles/loads clean, but a quick manual check there is worthwhile.


Open in Devin Review

Summary by CodeRabbit

  • New Features

    • Added type-checking to continuous integration workflow via new check script.
  • Bug Fixes

    • Resolved circular type reference issues in choice list components.
    • Fixed initialization timing in choice suggester.
  • Refactor

    • Updated command drag-handler event types across components.
    • Modified class property type declarations for consistency.
    • Streamlined global variable creation structure.

Review Change Stack

…k in CI

Makes `bunx svelte-check` clean (was 40 errors across 17 files) with type-only,
behavior-preserving changes, then adds it as a CI gate.

- dnd drag handles: retype `startDrag` from the wrong `(e: CustomEvent<DndEvent>)`
  to `(e: MouseEvent | TouchEvent)` (it is bound to on:mousedown/on:touchstart) in
  the 7 command components + CommandList; drop the now-unused DndEvent imports.
- Command subclasses: add `declare` to inherited name/id/type fields (TS2612;
  emission-free under the ES2020 target, and prevents field-clobbering if the
  target is ever raised).
- CommandList: narrow ICommand -> specific command types via <script> cast helpers
  passed one-way (`command={asWait(command)}`); children report edits via
  on:updateCommand events, so this is behavior-preserving. Use ConstructorParameters
  for the UserScriptSettingsModal settings arg.
- choiceList: break the ChoiceList <-> MultiChoiceListItem recursive type cycle
  with `as any` cross-imports; import ChoiceType from its canonical source; add a
  typed reorder handler; drop the dead `type="main"` prop.
- choiceSuggester: initialize choiceExecutor in the constructor body (after super)
  instead of as a field initializer, so `plugin` is defined (TS2729).
- GlobalVariables: drop a dead `id` field never present on the GV type.
- Add a `check` script (svelte-check) and run it in the CI Build + Lint job.

No Svelte version or bundling change. Verified: svelte-check 0/0, build-with-lint
clean, 1126 tests pass, plus a dev-vault smoke test (plugin reload, choice
suggester, settings render) with no runtime errors.

Closes #1241
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1676d84
Status:⚡️  Build in progress...

View logs

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces Svelte type-checking and refactors command/choice component architecture. It adds svelte-check validation, modernizes drag-handler event types across command components from DndEvent to native MouseEvent | TouchEvent, refactors CommandList to use type-narrowing helpers and one-way prop bindings instead of two-way binding, and resolves recursive type cycles in choice list components using type assertion workarounds.

Changes

Type-checking and component refactoring for Svelte

Layer / File(s) Summary
Svelte type-checking setup
package.json, .github/workflows/ci.yml
A new check script runs svelte-check --threshold error, and the CI build-lint job runs this check after build-with-lint.
Drag handler type modernization across command components
src/gui/MacroGUIs/Components/*.svelte
All command subcomponents (Wait, NestedChoice, UserScript, AIAssistant, OpenFile, Standard, Conditional) update their exported startDrag prop from (e: CustomEvent<DndEvent>) => void to `(e: MouseEvent
CommandList refactoring with type-narrowing and one-way props
src/gui/MacroGUIs/CommandList.svelte
Type-narrowing helpers (asWait, asNested, asUserScript, asAI, asOpenFile, asConditional) are introduced, command rendering changes from bind:command to one-way command={asX(command)} props, startDrag parameter type is modernized to MouseEvent | TouchEvent, and user-script settings extraction is refactored using a generic unknown shape.
Choice list recursive type cycle resolution
src/gui/choiceList/ChoiceList.svelte, src/gui/choiceList/MultiChoiceListItem.svelte
MultiChoiceListItem and ChoiceList imports are wrapped with as any casts to break the recursive type cycle detected by svelte-check, preserving runtime behavior.
ChoiceView reorder handler and wiring
src/gui/choiceList/ChoiceView.svelte
A new handleReorderChoices handler is added to persist reordered choices via saveChoices(), and ChoiceList event binding is updated to use the handler. The type="main" prop is removed from the ChoiceList component.
Global Variables data model update
src/gui/GlobalVariables/GlobalVariablesView.svelte
The addVariable function now creates new entries with only name and value fields, removing the id field while preserving unique name generation and persistence.
ChoiceSuggester initialization order fix
src/gui/suggesters/choiceSuggester.ts
The choiceExecutor field is moved from a class field initializer (which depended on this.plugin during construction) into the constructor body, ensuring correct initialization order.
Command type declarations converted to type-only
src/types/macros/ObsidianCommand.ts, src/types/macros/QuickCommands/AIAssistantCommand.ts, src/types/macros/QuickCommands/WaitCommand.ts, src/types/macros/UserScript.ts
The name, id, and type class properties are converted to declare-only fields (type-only, not runtime-emitted) in ObsidianCommand, AIAssistantCommand, WaitCommand, and UserScript, while runtime properties like commandId, time, and path remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

released

Poem

🐰 Svelte checks now catch our slips,
Drag events lose their DnD whips,
One-way props dance clean and bright,
Type cycles vanish in the night.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main objective: resolving svelte-check errors and enforcing it in CI, which aligns directly with the primary changes across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/svelte-check-types

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chhoumann chhoumann merged commit 0e6bfaa into master May 29, 2026
7 of 10 checks passed
@chhoumann chhoumann deleted the fix/svelte-check-types branch May 29, 2026 09:55
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant