Skip to content

Latest commit

 

History

History
215 lines (183 loc) · 15.7 KB

DECISIONS.md

File metadata and controls

215 lines (183 loc) · 15.7 KB

Design decisions

Durable rationale for choices that aren't obvious from the code alone - so they don't have to get re-argued or rediscovered in a future session. Add to this file when a decision was debated, has a non-obvious "why," or intentionally leaves something unsolved.

Scope: DevTools parity, not a "god-level terminal"

This project's REPL and console UI intentionally track feature-parity with Chrome DevTools' own console - not a superset of it. If DevTools doesn't have a piece of UX, we don't add it here, even when it looks like reasonable polish in isolation.

Concretely: an empty-state placeholder ("No logs yet...") was proposed, implemented, and then reverted at the maintainer's explicit request - DevTools has no equivalent, and the reasoning was "devs already know how to use a console." That's the standing bar for any future console-UX proposal: if you can't point to DevTools doing the same thing, don't add it, however small.

This does not limit visual/theming polish (animations, hover states, icon systems, scrollbar styling, resize affordances) - only new interaction patterns or explanatory UI aimed at a beginner audience this project isn't targeting.

REPL evaluation: reason over tokens, never over raw strings

Deciding what to do with typed REPL input - "is this an object literal or a block statement," "is this safe to live-preview without side effects" - used to be raw substring checks (code.startsWith('{'), code.includes('(')). That was rejected outright as brittle: a ( inside a string literal, or a => arrow, defeats a naive includes check; x++ mutates state but contains neither ( nor =, so it slipped through unnoticed.

The fix was not to add more special-cased string checks, and not to bring in a full parser/AST. It was to extract the single-pass scanner that already existed inside the syntax highlighter into a standalone, reusable lexer - src/runtime/tokenizer.ts

  • and make every REPL decision walk that token stream instead of the raw source string. Both syntax-highlighter.ts (rendering) and repl.ts (safety/disambiguation) now consume the same tokenize() output, so they can't drift out of sync with each other.

Why a lexer and not a full parser: a real parser (Acorn-class dependency, or a hand-rolled grammar) would let us ask "is this valid JS" and build a real AST, but the actual bugs we kept hitting were never "we lack a grammar" - they were blunt heuristics conflating syntax with semantics (a ( character isn't automatically a call; a keyword-shaped word isn't automatically a keyword when it follows .). A token-position check ("a ( only signals a call when the preceding token is an identifier, ), ], or ?.") solves that class of problem completely without the bundle-size cost of a real parser in a runtime meant to be embedded in a WebView. If a future feature genuinely needs AST-level understanding (auto-formatting, real static analysis), that's the trigger to revisit this - not another REPL safety heuristic.

Two related bugs already surfaced from getting this wrong once (fixed in the same PR that introduced the tokenizer, via self-review - see feat/ui-polish-expensive-look PR #14 commit 5cc36d4):

  • preEvaluate()'s keyword check was a blocklist. while (true) {} has no call, no assignment, no mutation - so it slipped through and was live-eval'd on a keystroke, hanging the page before Enter was ever pressed. Fixed by inverting to an allowlist of expression-safe keywords (typeof, void, this, in, instanceof) - any keyword not on that list fails closed, so a future keyword doesn't silently become previewable.
  • Reserved words used as property names (gen.return(), mod.default) were tokenized as keyword, which hid the call from the "is the token before ( callable" check and let a real method call run mid-typing. Fixed in the tokenizer: a word directly after . / ?. is always a property name, never a keyword.

Object-literal disambiguation matches Chrome's own trick

{a: 1} is ambiguous in JS: at the top of a statement it parses as a block (with a: read as a label), not an object literal, so plain eval throws Unexpected token ':'. Chrome's own console works around this by wrapping such input in parentheses to force expression context. We do the same: if the first significant token (skipping leading comments/whitespace) is {, try eval('(\n' + code + '\n)') first, and fall back to evaluating the raw source if that specifically throws a SyntaxError (a genuine block statement). The wrap is newline-padded so a trailing line comment ({a: 1} // note) can't comment out the closing paren.

A runtime error thrown while building the object literal (e.g. a getter that throws) is re-thrown as-is rather than triggering the fallback - only a SyntaxError from the wrap attempt falls back, so a real error from inside the literal is never silently re-executed a second time.

Known, accepted limitations (not bugs - don't "fix" these)

Some things are unsolvable by any static approach, including Chrome's own - these are documented so nobody spends time trying to close them:

  • A getter invoked via plain property access can't be distinguished from a plain property read without actually running it. preEvaluate('foo.expensiveGetter') will trigger that getter's side effects while the user is still typing, exactly like typing the same thing into Chrome's console does. There is no static analysis that solves this - the syntax is identical either way.
  • Tagged template calls (tag`hello`) aren't flagged as unsafe by isSafeToPreEvaluate(), since they involve no ( character at all. Rare enough in typical REPL usage not to be worth the added tokenizer/safety-check complexity.
  • Multi-line REPL input (src/runtime/ui.ts, the <textarea> REPL input) uses Shift+Enter for a newline and plain Enter to run, mirroring Chrome. Arrow-key history recall is line-aware: ArrowUp/ArrowDown only step through history when the caret is on the first/last line, so they move the cursor normally everywhere else in a multi-line command.

The public website (website/) is not one of the integration examples

website/ is a separate app from examples/* on purpose. The examples/* apps exist to prove each integration path works and stay minimal by design; a public-facing "why should I care" landing page has different goals (visual polish, a hero pitch, a live interactive demo area) and would have compromised the examples' minimalism if bolted onto one of them. It still uses the Vite plugin's early-injection method under the hood - that's the strongest proof of the project's core "survives your app crashing" pitch - and, like examples/published/*, installs the real npm package rather than local workspace source.

Two independently-versioned projects: the package and the website. Deploying the website used to be a side effect of publishing the package (deploy-demo ran needs: publish in release.yml, gated to stable tags). That meant a one-line copy fix on the site had no way to ship without also cutting a new package version - which is a false signal to npm consumers ("a new release landed") for a change that never touched the package at all.

Fixed by giving the website its own release line via vump's multi-project support - vump.toml now declares two [[project]] entries, main (tracking the root package.json, tagged v{version}, unchanged) and website (tracking website/package.json, tagged website-v{version}). Each tag shape triggers its own workflow: release.yml still publishes main to npm on a v* tag; the new release-website.yml deploys the website to Pages on a website-v* tag and nothing else. okcodes/vump/.github/actions/check reads the pushed tag and infers which project it belongs to from its shape, so neither workflow has to say --project explicitly. Shipping a site-only change is now vump patch --project website --tag --push - no package version, no npm publish, no changelog entry.

One consequence: a stable package release no longer auto-redeploys the website, so the version badge in its header can lag one release behind until the website is deployed again. That's a deliberate trade for dropping the second trigger path entirely, rather than keeping both wired into one shared deploy job.

The website shows its own version, fixed at a corner, not just a commit hash. Now that website is a real, independently-tagged project, a small VersionBadge renders v{website/package.json version}-{short commit sha} - e.g. v0.1.3-alpha.0-29d9871 - by importing website/package.json directly rather than adding a new build-time env var for it: the file is already the declared source of truth vump keeps in sync with the website-v* tag, so reading it directly can't drift from that the way a separately-computed value could. It moved out of the sticky nav (where it was sha-only before, and needed inline layout care to avoid crowding the npm/GitHub icons). It's fixed, not placed in the footer's normal document flow, on purpose - bug reports are screenshots, and this needs to show up in every one of them without the reporter scrolling to the bottom first.

No more pinning-with-retry. The old job pinned the website's dependency to the exact version its own run had just published (pnpm add @codehacks/virtual-console@<version>), retried because the registry hadn't necessarily propagated it yet. That race only existed because publish and deploy happened in the same run. Now they never do: release-website.yml just runs pnpm install --frozen-lockfile - the committed website/pnpm-lock.yaml is the declared input, not something the deploy job discovers or pins live. Same "Build inputs are declared, never discovered" principle as before, just resolved by removing the race instead of retrying around it.

pnpm-lock.yaml is committed here, unlike examples/published/*. That precedent's whole justification for an uncommitted lockfile is "float on @latest" for the one dependency that matters. website/ doesn't want that anymore - deploying is now decoupled from publishing (above), so nothing re-pins @codehacks/virtual-console at deploy time. An uncommitted lockfile would just let every dependency (that one included) drift unreproducibly on every install, local or CI. website/package.json itself pins the dependency to an exact resolved version rather than a latest alias, for the same reason - the file should say what's actually running, not defer to whatever a tag happens to resolve to at install time. Picking up a newer @codehacks/virtual-console is a deliberate, out-of-band step - pnpm bump:website:latest, or :alpha / :beta / :rc to preview a pre-release (a bare pnpm update can never reach those - a latest-alias/semver-range dependency only ever resolves to a stable version), committed like any other dependency bump, then a website release to actually deploy it - not something either workflow does on its own.

Requires the repo to be public. GitHub Pages is unavailable for private repos below a paid org plan, and an anonymous visitor can't open a private repo in StackBlitz either - there's no way to satisfy "one-click demo, no install" while the source stays private. The repo was made public for this reason (2026-08-26), which also required an org-level members_can_create_pages policy to be enabled - a broader-scoped, org-wide change that was called out and confirmed separately before touching it, since it wasn't implied by "make this one repo public."

The website prerenders to static HTML at build time, not just client-side React. Before this, dist/index.html shipped an empty <div id="root"> - fine for a browser, but a no-JS crawler (and some AI agents fetching the page) got nothing. Next.js was considered instead of fixing this in Vite, since SSR is its headline feature - but release-website.yml deploys to GitHub Pages, which only serves static files with no Node process behind it. Next's SSR/ISR/server-actions advantages need a server to run on, so on Pages it would run in output: 'export' (fully static) mode anyway - the same static-HTML output Vite already produces, just with the App Router's routing/data-fetching machinery for a site that has exactly one page and no react-router. That would have been a full framework migration for a capability Vite can add directly.

The fix: vite.ssr.config.ts builds src/entry-server.tsx (a renderToString(<App/>) wrapper) to dist-ssr/, and scripts/prerender.mjs inlines that HTML into dist/index.html's #root, then deletes dist-ssr/ - it's a build-time intermediate, not a deploy artifact. src/main.tsx now hydrates when #root already has children (a real build/preview load) and falls back to a normal createRoot().render() when it's empty (vite dev, which serves index.html unprerendered). vite.ssr.config.ts only carries the react() plugin, not virtualConsoleVitePlugin or tailwindcss() - the console plugin unconditionally emits its own client chunk on command === 'build', which breaks when pointed at a non-HTML SSR entry, and neither plugin's output (injected <script>/<style> tags, compiled CSS) is used by a build whose only output is an HTML string. release-website.yml needed no changes - pnpm run build still ends with static files in website/dist, just with real markup already in them.

No-JS visitors get a warning banner, kept outside the React tree on purpose. A <noscript> wrapping JSX children inside a hydrated React app is unreliable: React's hydration reconciliation ends up creating the children as real DOM nodes even with JS enabled (React uses createElement/ appendChild, not the HTML parser's noscript-content-as-text rule the initial static HTML got right by luck). The banner lives directly in index.html instead, so React never touches it and the browser's native noscript hiding is the only thing controlling its visibility. It's position: sticky so it stays visible while scrolling instead of disappearing with the rest of the page - the one warning on this site that matters most, since the whole point of the page is the console demo, and a visitor who can't get it to open needs to know why. A <style> scoped inside the same <noscript> (so it too only exists when scripting is disabled) sets Nav's <header> to position: static, ceding the sticky-top-0 spot to the banner - without it, both elements would fight over top: 0 while scrolling and visually overlap.

preview:website:watch rebuilds on save; no new dependencies for it. preview:website (see above) is a one-shot build-then-serve, deliberately with no watch mode - a full rebuild here is tsc -b plus two vite build passes plus the prerender script, several seconds of unrelated tooling to wire hot-reload around for something reached for occasionally, not run all day like dev:website. Once it's a full-rebuild-on-change loop instead of true HMR, though, the fix is a plain debounce-and-shell-out: scripts/watch-preview.mjs uses Node's built-in recursive fs.watch on src/ and index.html to re-run pnpm run build, alongside one long-lived vite preview process - which reads dist/ from disk per request, so it reflects each rebuild without restarting. No chokidar or concurrently added for this; a watch-and-shell-out loop this small doesn't need either.

Where to find the rest

  • README.md - installation, configuration, and the replEnabled: false escape hatch for shipping a zero-eval build.
  • examples/README.md - local workspace examples vs. standalone published-package examples.
  • website/README.md - the public website and how its deploy works.