Add debug tools - #177
Conversation
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
| v-bind="forwardedProps" | ||
| :class=" | ||
| cn( | ||
| 'origin-top-center bg-popover text-popover-foreground data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-90 relative mt-1.5 h-[var(--reka-navigation-menu-viewport-height)] w-full overflow-hidden rounded-md border shadow md:w-[var(--reka-navigation-menu-viewport-width)]', |
There was a problem hiding this comment.
are those vibe coded styles? 😆
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
This branch is significantly behind main and has merge conflicts. Notably, main already includes a more mature devtools implementation (PRs #356 and #398) with configurable ports, non-blocking connection handling, and performance gating via isDevtoolsEnabled(). If this PR is still relevant, it likely needs a full rebase and reconciliation with that existing work.
Core concerns if this continues:
Breaking API changes to memo() and effect(): Both functions changed their second parameter from a positional value (equal?: boolean / current?: T) to an options object. Any downstream consumer passing a boolean or value directly will break silently (the argument becomes the options object, which is truthy). These need either a migration path or a major version bump.
flushJobs() removed from render() entry point (render.ts ~line 188): Previously, effects queued during initial render were flushed before the print phase ran. Removing this changes execution semantics. Reactive updates that depend on synchronous flushing after render will now remain pending, potentially causing stale output. This is likely intentional (the scheduler refactoring moves toward explicit sync/post flush modes) but warrants a note in the PR description.
Debug server blocks module evaluation: The top-level await in debug.ts halts all downstream code until a browser client connects to port 8080. Any process that imports @alloy-js/core with ALLOY_DEBUG set will hang indefinitely if no client connects. The main branch implementation (devtools-server.ts) handles this more gracefully with timeouts and lazy initialization.
Memory growth in debug mode: nodeIds (Map<number, node>) holds strong references to every render tree node ever created, preventing GC. fileContents also grows without bounds. For long-running debug sessions this will eventually exhaust memory.
Stray console.log(filePath) in the HTTP static file handler (debug.ts). Should be removed or gated behind a verbose flag.
Commit hygiene: All 4 commits are "wip" or "rename". If this continues, an interactive rebase into atomic, descriptive commits would help reviewability.
| } | ||
|
|
||
| export function memo<T>(fn: () => T, equal?: boolean): () => T { | ||
| export interface MemoOptions<T> extends ReactiveEffectOptions<T> { |
There was a problem hiding this comment.
The signature change from memo(fn, equal?: boolean) to memo(fn, options?) is a breaking change. Callers passing true as the second arg will now get options = true which won't match options.equal. Consider an overload or deprecation period.
| } | ||
| trace( | ||
| TracePhase.scheduler.flush, | ||
| () => `flushing jobs, queue size: ${queue.size}`, |
There was a problem hiding this comment.
The flushing guard prevents re-entrant flushes, which is good. But if an effect queues a new job during flush, that job won't run until the next explicit flushJobs() call. Previously the loop would naturally pick it up since takeJob() kept pulling from both queues. Worth confirming this doesn't cause stale renders in practice.
3 participants
This PR introduces debug tools. With these tools you can:
As part of this PR, some additional capabilities were added and bugs fixed.