refactor: improve signals APIs, add `effect` and `computed` helpers by nberlette · Pull Request #6 · nberlette/tui

nberlette · 2026-03-11T18:34:52Z

refactor(signal): improve errors in signal submodules
feat(utils): add internal performance module for mocking
feat(internal): add internal types and primordials modules
chore: fmt
refactor(signals): clean various little things up
Document flusher usage
refactor(signals): clean up formatting, use primordials more consistently

- [x] add a dedicated ./signals/errors submodule - [x] move error subclasses out of signal.ts and computed.ts - [x] improve some other minor things

…ntly

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 176b64f573

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-11T18:39:21Z

src/signals/dependency_tracking.ts

    incoming = 0;
    throw error;
+  } finally {
+    activeSignals = undefined;
+    --incoming;


Keep incoming counter non-negative on tracking errors

If func.call(thisArg) throws, the catch block sets incoming = 0 but the finally block still executes --incoming, which leaves the counter at -1. Subsequent calls then block forever in while (incoming) await Promise.resolve(); because -1 is truthy, so one tracking-time exception can deadlock all future dependency tracking (Computed, Effect, etc.) in the same process.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR refactors the signals subsystem and related utilities to provide cleaner APIs (effect, computed, isSignal), centralize error types, and standardize usage of “primordials”/global utilities across runtime environments.

Changes:

Introduces new internal utility modules (primordials, performance, types) and applies them in stdio wrappers.
Refactors signals internals (dependency tracking, errors, type helpers) and adds convenience helpers (effect(), computed(), isSignal()).
Improves documentation/formatting across signals modules (lazy computed/effect, flusher, reactivity).

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
src/utils/types.ts	Adds `Reshape<T>` helper type for reuse.
src/utils/primordials.ts	Adds `$global` and “SafeObject” wrappers for Object primordials.
src/utils/performance.ts	Adds a mockable `performance.now()` abstraction.
src/stdio/stdout.ts	Switches to primordials-backed `ObjectAssign`/`ObjectDefineProperty`.
src/stdio/stdin.ts	Uses shared `Reshape` type + primordials-backed Object helpers; doc tweak.
src/stdio/stderr.ts	Switches to primordials-backed `ObjectAssign`/`ObjectDefineProperty`.
src/signals/types.ts	Adds `Dependish`, `Disposable`, `AsyncDisposable` types and minor formatting.
src/signals/signal.ts	Centralizes error import, tightens options typing, adds `isSignal`/`SignalLike`, refactors internals.
src/signals/reactivity.ts	Minor refactor/formatting of propagation conditions.
src/signals/mod.ts	Re-exports new `errors.ts`.
src/signals/lazy_effect.ts	Uses new `performance` util; exports options type; doc/cleanup.
src/signals/lazy_computed.ts	Uses new `performance` util; exports options type; doc/cleanup.
src/signals/flusher.ts	Documentation improvements and minor loop simplification.
src/signals/errors.ts	New centralized error types for signals modules.
src/signals/effect.ts	Improves error throwing, pause/resume semantics, adds `effect()` helper.
src/signals/dependency_tracking.ts	Renames internals to `track`/`optimize` and re-exports old names.
src/signals/computed.ts	Moves error to `errors.ts`, minor cleanup, adds `computed()` helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T18:41:18Z

src/signals/computed.ts

+export function computed<T>(computable: Computable<T>): Computed<T> {
+  return new Computed(computable);
+}


New public helper computed() is added here but existing signal tests only cover constructing Computed directly. Add a small test that imports and uses computed() to ensure it’s exported correctly and behaves the same (read-only behavior + dependency tracking).

Copilot · 2026-03-11T18:41:18Z

src/utils/primordials.ts

+export const $global: typeof globalThis = (() => {
+  try {
+    return (0, eval)("this");


This new file is missing the standard license header comment used across the repo (many existing src/**/*.ts files start with a copyright + MIT license line). Please add the header so it passes the repository’s license-header checks.

Copilot · 2026-03-11T18:41:19Z

src/utils/performance.ts

+import { $global } from "./primordials.ts";
+
+export interface Performance {


This new file is missing the standard license header comment used across the repo (many existing src/**/*.ts files start with a copyright + MIT license line). Please add the header so it passes the repository’s license-header checks.

Copilot · 2026-03-11T18:41:19Z

src/signals/dependency_tracking.ts

  try {
    func.call(thisArg);
  } catch (error) {
    incoming = 0;
    throw error;
+  } finally {
+    activeSignals = undefined;
+    --incoming;
  }


In track(), the catch sets incoming = 0 but the finally block always decrements incoming, so an exception will leave incoming negative (e.g. -1). This can break the while (incoming) gating and dependency tracking state. Remove the incoming = 0 assignment (or move it into finally without also decrementing) so incoming is always restored to its previous value.

Copilot · 2026-03-11T18:41:19Z

src/stdio/stdout.ts

+      {
+        __proto__: impl,
+        get() {
+          if (impl && "writable" in impl) {
+            return impl.writable;
+          } else {


The property descriptor passed to ObjectDefineProperty sets __proto__ to impl, which changes the descriptor object's prototype to a user-provided object. This is unnecessary for defining an accessor and can introduce surprising prototype-chain lookups / security footguns. Drop __proto__ entirely (or set it to null if you want a dictionary object).

Copilot · 2026-03-11T18:41:19Z

src/utils/types.ts

@@ -0,0 +1 @@
+export type Reshape<T> = Pick<T, keyof T>;


This new file is missing the standard license header comment used across the repo (e.g. // Copyright 2023 Im-Beast. MIT license. at the top of other src/utils/*.ts files). Please add the header so it passes the repository’s license-header checks.

Copilot · 2026-03-11T18:41:20Z

src/signals/errors.ts

+/** Thrown whenever someone tries to directly modify `Computed.value` */
+export class ComputedReadOnlyError extends Error {
+  constructor() {


This new file is missing the standard license header comment used across the repo (many existing src/signals/*.ts files start with a copyright + MIT license line). Please add the header so it passes the repository’s license-header checks.

Copilot · 2026-03-11T18:41:20Z

src/signals/signal.ts

+        // If dependant has no more dependencies then
+        // it means that it should be replaced with constant value,
+        // because nothing can update its value anymore
+        if (!dependant.dependencies) dependant.dispose();


dependant.dependencies is always a Set per the Dependant interface, so if (!dependant.dependencies) will never be true. If the intent is to dispose dependants that no longer depend on anything, this should check dependant.dependencies.size === 0 after deleting this signal.

Suggested change

if (!dependant.dependencies) dependant.dispose();

if (dependant.dependencies.size === 0) dependant.dispose();

Copilot · 2026-03-11T18:41:20Z

src/stdio/stderr.ts

+      {
+        __proto__: impl,
+        get() {
+          if (impl && "writable" in impl) {
+            return impl.writable;
+          } else {


The property descriptor passed to ObjectDefineProperty sets __proto__ to impl, which changes the descriptor object's prototype to a user-provided object. This is unnecessary for defining an accessor and can introduce surprising prototype-chain lookups / security footguns. Drop __proto__ entirely (or set it to null if you want a dictionary object).

Copilot · 2026-03-11T18:41:20Z

src/signals/effect.ts

+export function effect(effectable: Effectable): Effect {
+  return new Effect(effectable);
+}


New public helper effect() is added here but existing signal tests only cover constructing Effect directly. Add a small test that imports and uses effect() to ensure it’s exported correctly and behaves the same (dependency tracking, pause/resume/dispose).

nberlette added 7 commits March 9, 2026 18:24

refactor(signal): improve errors in signal submodules

fc00e78

- [x] add a dedicated ./signals/errors submodule - [x] move error subclasses out of signal.ts and computed.ts - [x] improve some other minor things

feat(utils): add internal performance module for mocking

bb505fa

feat(internal): add internal types and primordials modules

f41fda1

chore: fmt

57df2f4

refactor(signals): clean various little things up

501e914

Document flusher usage

2770675

refactor(signals): clean up formatting, use primordials more consiste…

176b64f

…ntly

Copilot AI review requested due to automatic review settings March 11, 2026 18:34

nberlette added the feature New feature or request label Mar 11, 2026

nberlette self-assigned this Mar 11, 2026

Copilot started reviewing on behalf of nberlette March 11, 2026 18:35 View session

nberlette merged commit ac71068 into refactor/components Mar 11, 2026
2 of 4 checks passed

nberlette deleted the refactor/signals branch March 11, 2026 18:36

chatgpt-codex-connector bot reviewed Mar 11, 2026

View reviewed changes

Copilot AI reviewed Mar 11, 2026

View reviewed changes

		import { $global } from "./primordials.ts";

		export interface Performance {

	if (!dependant.dependencies) dependant.dispose();
	if (dependant.dependencies.size === 0) dependant.dispose();

Uh oh!

Conversation

nberlette commented Mar 11, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants