main retired electrobun's native context menu (a documented no-op on Linux) in favour of an HTML one on every platform. Merging that here needs the menu to look right under the Win7 skin as well. Conflicts, all resolved by keeping both sides: - src/bun/index.ts — createAppRPC keeps the branch's mainWindow getter, drops main's removed contextMenu dep. - ui/src/app/app.ts — imports both TitleBar and ContextMenu. - ui/src/app/platform.ts — kept the branch's version (its IS_MAC_CHROME accounts for the frameless win7 chrome, and MOD_LABEL correctly keys on the real platform rather than the chrome), plus exporting IS_MAC and adding IS_LINUX for the menu's per-OS shell wording. Aero menu (branch-only): 7.css has no menu component, so it is hand-authored across both layers — the row <button>s shed their push-button slab in `skin` (revert-layer only reaches `utilities` from there), and the square-cornered slab, hairline border and gradient selection band are stated in `skinfix`. Built strictly from existing --w7-* vars, never literal hex, so the hand-written dark Win7 palette recolours it for free. Hover and the keyboard cursor share one band (the component marks the cursor with `bg-accent`, which is overridden alongside :hover) as a real Win7 menu does. Also answers a question the merge raised: frameless windows DO work on Linux. The GTK wrapper honours titleBarStyle "hidden" — it calls gtk_window_set_decorated(false) and logs "GTK: Created window without decorations (custom titlebar)". Verified on GNOME/Wayland and documented in architecture.md. Verified: backend `tsc --noEmit` clean, `bun run build:ui` clean (492.52 kB initial, still under the 500 kB budget), app launches with the skin active, frameless, no console errors and no GBM failures. Co-Authored-By: Claude <noreply@anthropic.com>
192 lines
12 KiB
Markdown
192 lines
12 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`.
|
||
- **The win7 skin layer outranks Tailwind utilities.** 7.css is imported
|
||
`layer(skin)`, ordered above `utilities`, so any utility you add to a
|
||
`button`/`input`/`select`/`textarea`/`table` is silently dead under that
|
||
skin. If it matters, neutralize in the `@layer skin` block at the bottom of
|
||
styles.css — and note `revert-layer` only reaches `utilities` from **inside
|
||
`skin`**; used from `skinfix` it falls back onto 7.css and is a no-op. This
|
||
bit once already.
|
||
- `@import … layer(name)` works through `@tailwindcss/postcss` (it wraps the
|
||
imported AST), but the **deep path is mandatory**: 7.css ships no `exports`
|
||
map and Tailwind's resolver reads only `style`/`mainFields`, so a bare
|
||
`@import "7.css"` resolves to nothing. Use `7.css/dist/7.scoped.css`.
|
||
- Stylesheet `url()` must resolve at BUILD time: assets referenced from
|
||
`styles.css` live under `ui/src/` (esbuild emits them hashed into `media/`
|
||
and rewrites the url), NOT `ui/public/` — a public path fails the build with
|
||
"Could not resolve".
|
||
|
||
## 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.showContextMenu` is a silent no-op on Linux** (electrobun's own
|
||
`dist/api/bun/proc/linux.md` says so; the app also prints "Application menus are
|
||
not supported on Linux" at boot). Because the UI suppresses the webview's own
|
||
right-click menu, that left EVERY panel context menu dead on Linux — no error,
|
||
nothing happens. The menu is plain HTML now (`ui/src/app/context-menu.ts`) on
|
||
every platform; don't reintroduce the native one for "nativeness" on mac/win
|
||
without keeping a Linux path.
|
||
|
||
## Linux
|
||
|
||
- **`ERR_DLOPEN_FAILED` on `bun run start`** means a missing system library, not
|
||
broken app code. `libNativeWrapper.so` links `libayatana-appindicator3.so.1`
|
||
*hard* (for the tray), so the app cannot start at all without it — even though
|
||
the tray is a minor feature. Diagnose with
|
||
`ldd build/dev-linux-x64/*/bin/libNativeWrapper.so | grep 'not found'`; the
|
||
`libasar.so => not found` line in that output is a red herring (it ships beside
|
||
the wrapper and resolves at load). Install list per distro is in
|
||
[platform-notes.md](platform-notes.md).
|
||
- **Black window on NVIDIA + `Failed to create GBM buffer of size WxH: Invalid
|
||
argument`** (usually with `X11 Error: GLXBadWindow (code 170)`). The app boots,
|
||
logs "miniSCP started", reports no error — and paints nothing. WebKitGTK's
|
||
DMABUF renderer can't allocate buffers on the NVIDIA proprietary driver;
|
||
`WEBKIT_DISABLE_DMABUF_RENDERER=1` is the upstream workaround.
|
||
- **The app cannot apply this to itself.** WebKit reads the variable when it
|
||
forks its web process, and **Bun's `process.env` writes do not propagate to
|
||
child processes** (verified: a spawned child sees it unset). Setting it in
|
||
`src/bun/index.ts` — even in a module imported before `electrobun/bun` —
|
||
does nothing. It must be in the environment before the app process starts.
|
||
- Hence every launch path injects it: `scripts/linux-gpu.ts` (`launchEnv()`)
|
||
for `bun run dev` / `bun run start`, and the `miniSCP.sh` wrapper that
|
||
`release.ts` writes beside `bin/launcher` in the tarball. **A new launch
|
||
path must call `launchEnv()` or it will render black on NVIDIA.**
|
||
- Gated on `/proc/driver/nvidia` so Intel/AMD keep hardware compositing, and
|
||
an explicit `WEBKIT_DISABLE_DMABUF_RENDERER` from the user always wins.
|
||
- A "does it boot?" log check does NOT catch this — grep for the GBM line, or
|
||
look at the window.
|
||
- Benign startup noise on Linux — non-fatal, the app runs normally:
|
||
`libayatana-appindicator is deprecated. Please use libayatana-appindicator-glib`
|
||
(upstream electrobun's choice, not ours).
|
||
- **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.
|
||
The win7 skin adds a **fourth value to keep in sync**: `buildPreload` also
|
||
bakes the `.win7` class, and index.html carries a matching `html.win7` arm
|
||
for all four theme states. Miss it and every win7 window flashes white.
|
||
- **`titleBarStyle` is construction-time only.** Switching skin cannot
|
||
re-frame a live window; `chromeOptions` is read once per `new
|
||
BrowserWindow`. The options row tells the user to reopen. Don't "fix" this
|
||
by recreating the main window — that would drop every session and tab.
|
||
- 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.
|
||
|
||
## i18n
|
||
|
||
- **Translations live in `src/shared/i18n/*.ts` (TS, not JSON)** — deliberate:
|
||
it sidesteps `resolveJsonModule` differences between the Angular esbuild and
|
||
Bun builds and gives full typing. `en.ts` is the source of truth; other
|
||
locales are partial and fall back to English then the raw key.
|
||
- **`i18n.t()` is reactive because it reads the `locale` signal** — that's what
|
||
makes live language switching work with no pipe. If you cache a translated
|
||
string in a plain field it WON'T update on switch; call `t()` in the template
|
||
(or in a signal-reading getter) instead. Same for `i18n.date()/size()`.
|
||
- Dictionary values are TS string literals: **French/other apostrophes break
|
||
single-quoted entries** (`'…c'est…'`). Use double quotes for any value
|
||
containing an apostrophe. Plurals are `{ one, other }` maps (not ICU strings);
|
||
zh/ja supply only `other`.
|
||
- **Native menus can't use the Angular service** — `menu.ts` goes through backend
|
||
`i18n.ts`, and the app menu must be rebuilt when the locale changes (wired in
|
||
index.ts `settingsChanged`), or it keeps the old language. The context menu no
|
||
longer has this problem: it is Angular now and reads `I18nService` directly.
|
||
- `eu`/`zh`/`ja` are AI drafts pending native review (see the i18n README).
|
||
- **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.
|