jgamboaandClaude Fable 5
ee5a9e13e6
Local panels are fs.watched (LocalDirWatcher + setWatchedDirs -> pathRefresh);
remote panels poll every 3s. New refresh({silent}) skips the loading/busy
flags and touches no DOM, snapshotting sessionId+cwd to discard results if the
user navigated mid-flight. Only the active tab is live; switching tabs
refreshes the new pair. A future on/off setting is left as commented
breadcrumbs. Docs updated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
111 lines
6.5 KiB
Markdown
111 lines
6.5 KiB
Markdown
# Gotchas — landmines this codebase has already stepped on
|
||
|
||
Every one of these caused a real bug or debugging spiral. Read before
|
||
touching the adjacent code.
|
||
|
||
## Angular (v22, zoneless, signals, OnPush)
|
||
|
||
- **Never consume push streams through signal+effect.** An effect that reads
|
||
a signal and (directly or transitively) writes signals re-triggers itself —
|
||
this froze the entire app on the first transfer. Push messages
|
||
(transferUpdate etc.) go through plain callback registries in RpcService.
|
||
Signal coalescing would also silently drop intermediate progress snapshots.
|
||
- **No signal writes during template render** (NG0600). Anything that loads
|
||
panels from code paths reachable during render defers with
|
||
`queueMicrotask` (see `PanelsService.newWorkspace`).
|
||
- **`[value]` on `<select>` is applied before `@for` options render** and
|
||
silently selects nothing. Use per-option `[selected]` bindings (connection
|
||
manager stack picker).
|
||
- Template bindings with special characters: `[class.bg-primary/15]` works;
|
||
`[class.shadow-[...]]` (brackets) does NOT parse — use `[style.box-shadow]`.
|
||
Static class attributes may use arbitrary values freely.
|
||
- Functions called from templates ARE reactive: signals read inside them
|
||
register on the view's consumer (call-stack-based tracking). That's why
|
||
`iconClass(entry)` reacting to `settings.iconColors()` works.
|
||
- Dedicated tools > shell: IDE diagnostics between paired template/class
|
||
edits are often stale mid-sequence — trust `bun run build:ui` (full AOT
|
||
type-check), not the first red squiggle.
|
||
- Tailwind v4 canonical class names: `wrap-break-word` not `break-words`,
|
||
`accent-primary` not `accent-[var(--primary)]`, `z-60` not `z-[60]`.
|
||
Every color used in a template must resolve through `@theme inline`.
|
||
|
||
## Electrobun
|
||
|
||
- **Window `close` is not cancellable** (response type `{}`; only the
|
||
app-level `before-quit` supports `event.response = {allow:false}`). All
|
||
unsaved-state guards must either intercept app-controlled close paths
|
||
(⌘W, buttons) or survive the close (editor draft mirroring).
|
||
- **One rpc instance per window** — BrowserView binding steals the rpc's
|
||
transport; a shared instance silently sends to the wrong webview.
|
||
- **RPC before socket open is silently lost.** Everything gates on the
|
||
socket-open promise (RpcService `ready`).
|
||
- The emitter has no `off()`. Window event closures must not capture records
|
||
you want garbage collected.
|
||
- `electrobun build` defaults to the **dev** channel; release builds need
|
||
`--env=stable`. The stable app name differs from the dev one
|
||
(`miniSCP` vs `miniSCP-dev`), including the app-data dir implications when
|
||
testing.
|
||
- Tray: never call `setImage` after construction on macOS — the template
|
||
treatment is lost and the icon renders black on dark menu bars.
|
||
- `ApplicationMenu` is macOS-only in our usage; accelerators on Windows are
|
||
single-character only. ContextMenu exists on macOS+Windows, not Linux.
|
||
- **Window-open flash prevention is three-layer** — remove any one and the
|
||
white/inverted blink returns: (1) windows are created `hidden: true` and
|
||
revealed by `showWhenReady` on the webview's first `dom-ready` (800 ms
|
||
fallback), (2) the bun preload bakes the persisted `data-theme`/tone/accent
|
||
attributes before first paint (`buildPreload`), (3) `ui/src/index.html`
|
||
carries an inline `<style>` with the html background per theme — its oklch
|
||
values MUST stay in sync with the `--background` tokens in styles.css.
|
||
- Dev vs prod URL: `getMainViewUrl()` resolves the ng-serve dev server only
|
||
when `DEV_SERVER_URL` is set AND the updater channel is `dev` AND the
|
||
server answers a HEAD request; secondary windows must reuse the resolved
|
||
URL, never hardcode `views://`.
|
||
- Upstream Bun panic (`FFI_Callback_threadsafe_call` → `web_worker.spin`):
|
||
rare segfault under long sessions; upstream electrobun×Bun issue, restart
|
||
recovers, nothing app-side to fix.
|
||
|
||
## Domain logic
|
||
|
||
- **sessionId ≠ filesystem identity.** Two panels on the same server are two
|
||
SSH sessions. Every same-file/nested-path guard compares `fsKey`
|
||
(`resolvedIP:port` / `"local"`); comparing sessionId caused a
|
||
truncate-to-zero data-loss bug. When fsKeys differ but paths collide, the
|
||
probe-file check unmasks aliased hosts. Never "simplify" this back.
|
||
- `createWriteStream` truncates BEFORE the source is read — that's why
|
||
self-copy is destructive, and why guards must run before any stream opens.
|
||
- The synthetic `'..'` row: type `'dir'`, must be excluded from selection,
|
||
targets(), filtering match-count, kind/rights/owner display, and drag
|
||
sources. Any new row-level feature must decide what `'..'` does.
|
||
- `Semaphore.wake()` must reserve slots synchronously; waking waiters that
|
||
increment asynchronously loses budget under concurrent release.
|
||
- Settings: the backend spread-merges (`{...DEFAULT_SETTINGS, ...data}`) so
|
||
new keys auto-default, but the UI SettingsService reads field-by-field —
|
||
a new setting silently stays at default until wired in all five spots.
|
||
- The editor draft mirror sends full content per tick (2 MB cap upstream);
|
||
don't shrink the 400 ms debounce.
|
||
- `panel.touched` gates localDir auto-navigation; `connectToken` guards
|
||
against a slow connect resolving into a panel the user already reused.
|
||
- **Silent auto-refresh must snapshot sessionId+cwd and discard a mismatched
|
||
result.** A normal `refresh()` is masked by `busy` (input blocked), but the
|
||
silent variant (poll/watch) runs while the user can navigate — without the
|
||
guard, a poll that started in dir A resolves and stamps A's listing over the
|
||
dir B the user just opened. It also never calls `scrollCursorIntoView`
|
||
(scroll must stay put) and returns early if `busy()` or already refreshing.
|
||
|
||
## Build / tooling
|
||
|
||
- Angular CLI rejects Bun's Node-compat version — `scripts/node-version-shim.ts`
|
||
is preloaded via `bunfig.toml` at the root AND in `ui/`. Removing either
|
||
breaks `ng`.
|
||
- ssh2's optional native deps must stay `external: ["cpu-features", "*.node"]`
|
||
in `electrobun.config.ts` — bundling them breaks the build; ssh2 falls back
|
||
to pure JS at runtime.
|
||
- `bun run --cwd ui build` (flag order matters; `bun --cwd ui run build` does
|
||
not do what you want).
|
||
- `version.gen.ts` is gitignored and regenerated by postinstall + every build
|
||
script; a fresh checkout fails `tsc` until `bun install` (or any script)
|
||
has run once. Never commit it — it would bump the commit count it records.
|
||
- Icon pipeline is `scripts/gen-icons.ts` (resvg) — regenerate after touching
|
||
`assets/logo.svg` or `assets/tray/tray-glyph.svg`; the .ico and tray PNGs
|
||
are build inputs, not build outputs.
|