Code Review: app-launcher.ts - Type Safety and Declarative Patterns

[johnlindquist]

Overview

Code review of app-launcher.ts from React Core and TypeScript (Anders Hejlsberg) perspectives reveals critical type safety issues and imperative patterns that hurt maintainability.

🔴 Critical Issues

1. Type Safety Violations (Line 73)

Problem: as any cast completely bypasses TypeScript's type system

appsDb.choices as any  // Line 73

Impact:

• Destroys IDE autocomplete and refactoring capabilities
• Runtime errors become inevitable when shape of choices changes
• No IntelliSense or safety net

Solution: Type appsDb properly:

interface DbResult<T> {
  choices: T[]
}

// Option 1: Type the result
const appsDb: DbResult<Choice> = await db(...)

// Option 2: At minimum
appsDb.choices as Choice[]  // with comment explaining why

2. Implicit Any Types (Lines 48, 61)

Problem: Multiple implicit any types throughout:

• input => { ... } (line 61) - what's the type of input?
• Flag values have no type definition

Solution: Define a Flags interface:

interface AppLauncherFlags {
  prep?: boolean
  input?: string
  refresh?: boolean
}

🟡 Major Concerns

3. Imperative State Mutations (Lines 24-35)

Problem: Direct mutation of UI state through multiple imperative calls:

setResize(true)
setChoices([{ name: "...", info: true }])
clearTabs()
setPlaceholder("One sec...")

React perspective: This is the antithesis of declarative UI. State changes are scattered and unpredictable with no single source of truth.

Solution: Use declarative state pattern:

const loadingState = {
  resize: true,
  placeholder: "One sec...",
  choices: [{ name: "First Run: ...", info: true }],
  tabs: []
}

4. Side Effect Chaos (Lines 17-18)

Problem: Side effect triggered by conditional without clear data flow:

if (!flag.prep) {
  preload()  // Hidden dependencies and mutations
}

React principle: Side effects should be explicit, predictable, and well-contained.

5. Unclear Data Flow (Lines 21-43)

Problem: The db() function pattern obscures actual data flow:

let appsDb = await db(
  kitPath("db", "apps.json"),
  async () => { /* side effects + data loading */ },
  !flag?.refresh  // Cache control via boolean
)

Issues:

• Is this a cache? A lazy loader? Both?
• Second argument is a factory function with side effects
• Third argument inverts cache logic (!flag?.refresh is confusing)
• No way to predict when UI updates happen

6. Missing Type Guards (Line 75)

Problem: Check for app without type narrowing:

if (app) {
  open(app)
}

What type is app? Could be string | Choice | undefined. Without proper typing, open(app) might fail at runtime.

🟢 Moderate Issues

7. Magic Strings and Stringly-Typed Code (Lines 48-73)

Problem: String literals everywhere without type safety:

key: "app-launcher"  // Line 50
kitPath("main", "app-launcher.js")  // Line 64
"--input", "--refresh"  // Lines 65, 67

Solution:

enum AppLauncherKey {
  Key = "app-launcher",
  ScriptPath = "main/app-launcher.js"
}

enum Flag {
  Input = "--input",
  Refresh = "--refresh",
  Prep = "--prep"
}

8. Poor Discoverability (Lines 54-71)

Problem: Shortcuts object is nested and hard to discover.

Solution: Extract to well-typed constant:

interface Shortcut {
  name: string
  visible: boolean
  key: string
  bar: 'left' | 'right'
  onPress: (input: string) => Promise<void>
}

const REFRESH_SHORTCUT: Shortcut = { ... }

9. Platform Check Without Abstraction (Lines 8-15)

Problem: Early exit for Linux without abstraction:

if (isLinux) {
  await div(md("..."))
  exit()
}

Better: Extract to typed PlatformCheck function that returns Result<void, UnsupportedPlatformError>.

🔵 Minor Observations

10. Inconsistent Async Handling

• Line 18: preload() called without await (fire-and-forget?)
• Lines 21, 48, 63, 76: Other async calls use await

Is preload() intentionally fire-and-forget? If so, add a comment.

11. Function Call Without Import Context

Lines call global functions (div, md, exit, preload, setResize, arg, open) without imports. Add reference comment:

// Global runtime functions injected by Script Kit
// See: sdk/src/globals/...

✅ What's Good

1. Clear intent - Script's purpose is obvious from comments and structure
2. User feedback - Good loading messages for first-run experience (lines 27-29)
3. Caching strategy - The db() pattern provides reasonable caching (line 42)
4. Platform awareness - Correctly checks isLinux early (line 8)

📋 Recommendations

Immediate (P0):

1. Remove as any - Type the appsDb.choices properly
2. Define Flags interface - Make flag types explicit
3. Type the onPress callback - What's the type of input?

Short-term (P1):

4. Extract UI state - Create a LoadingState type describing the full UI
5. Document globals - Add a comment explaining the injected runtime
6. Add type guard for app - Narrow the return type of arg()

Long-term (P2):

7. Refactor to declarative pattern - Consider modeling this as state + reducer
8. Extract constants - Replace magic strings with typed enums
9. Create Shortcut type - Make shortcuts discoverable and type-safe

🎯 Severity Summary

High Impact:

• Type safety (as any cast) - Breaks tooling, invites runtime errors
• Imperative mutations - Makes code hard to reason about

Medium Impact:

• Missing type definitions - Hurts developer experience
• Unclear data flow - Maintenance burden

Low Impact:

• Magic strings - Refactoring friction
• Documentation gaps - Onboarding friction

❓ Questions

1. What is the actual return type of db()? Can we type it properly?
2. Is preload() intentionally fire-and-forget?
3. Where are setResize, setChoices, etc. defined? Can we import them?
4. What's the type contract for the arg() function's return value?
5. Why !flag?.refresh instead of flag?.refresh === true for clarity?

Final Verdict

• TypeScript grade: C- - Types exist but are actively bypassed with as any
• React principles grade: D+ - Imperative mutations dominate; no clear data flow

This code works, but it's fighting TypeScript instead of leveraging it, and it's imperative where it could be declarative. With proper typing and state modeling, this could be excellent.