From f05096aac0a48758183f11637b42bf34449051fb Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 10:12:29 -0400 Subject: [PATCH 1/2] Add plan: modernize @tko/utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phased modernization of @tko/utils — drop legacy polyfill probes, tighten types, preserve public API surface. Five independently shippable phases (A–E). Phase A is already implemented on modernize/utils-dead-polyfills and green on both browser and happy-dom projects. Out of scope: @tko/lifecycle refactor (#322), public API removal, the deferError/onError swallowing bug (separate branch). Co-Authored-By: Claude Opus 4.7 (1M context) --- plans/modernize-utils.md | 130 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 plans/modernize-utils.md diff --git a/plans/modernize-utils.md b/plans/modernize-utils.md new file mode 100644 index 000000000..190fb1105 --- /dev/null +++ b/plans/modernize-utils.md @@ -0,0 +1,130 @@ +# Plan: Modernize `@tko/utils` + +**Risk class:** LOW–MEDIUM — each phase is independently shippable and touches a +leaf package. Public API surface is preserved per phase. +**Owner:** brianmhunt + +## Progress + +| Phase | Status | PR | Notes | +|-------|--------|-----|-------| +| A. Dead polyfill probes | Ready | TBD | symbol, function, string, css — drop legacy shims. Net -40 lines. Branch `modernize/utils-dead-polyfills` already built and green (2698 browser + 2671 happy-dom tests). | +| B. tasks.ts microtask | Not started | — | Drop MutationObserver fallback; `queueMicrotask` is universally available. | +| C. object.ts | Not started | — | Adopt `Object.hasOwn` (ES2022); delete `@deprecated clonePlainObjectDeep`. | +| D. memoization.ts | Not started | — | Replace prototype-exposed `{}` store with `Map`. | +| E. array.ts idioms | Not started | — | Kill `arguments.length > 2 ? bind(thisArg)` pattern; inline native methods; standardise `[]` over `new Array()`. Public export names preserved. | +| F. (out-of-band) deferError bug | Spawned task | — | `deferError` routes through `safeSetTimeout`, so `options.onError` swallows errors meant to be surfaced. Fixed on a separate branch, not part of this plan. | + +## Context + +`@tko/utils` is the leaf dependency of almost every other TKO package. Parts of +it pre-date ES2015 as a baseline: + +- `useSymbols = typeof Symbol === 'function'` (always `true` now) +- `functionSupportsLengthOverwrite` runtime probe (ES6 made `Function.length` + configurable universally) +- `stringTrim` IE `\xa0` regex fallback (`String.prototype.trim` is ES5) +- `stringStartsWith` polyfill (ES6 native) +- `toggleDomNodeCssClass` SVGAnimatedString and string-className fallbacks + (all `Element`s have `classList`) +- `tasks.ts` MutationObserver fallback (`queueMicrotask` since Safari 12.1 / 2018) +- `clonePlainObjectDeep` — flagged `@deprecated Function is unused` +- Plain-object dictionary stores with string keys (prototype pollution surface) +- `new Array()`, `substring`, `.apply(null, args || [])` — pre-spread idioms + +The AGENTS.md rule "TKO is perf-sensitive — keep function bodies lean for +inlining" applies here; most modernization here reduces branching inside +hot-path utilities. + +## Scope + +**In scope.** Everything listed in the phase table. Public function names, +signatures, and documented behaviour are preserved. Types may be tightened +(e.g. `stringTrim(value: unknown): string` over the inferred `any`). + +**Out of scope.** + +- The `@tko/lifecycle` refactor (issue #322 — composition over mixins, + `Symbol.dispose`). That is a separate, larger plan. +- Any public API removal. If a consumer depends on a function name today, it + still works after this plan. +- The `deferError` swallowing bug — fixed on its own branch to keep this plan's + scope surgical. + +## Phases + +### Phase A — Dead polyfill probes + +Files: `symbol.ts`, `function.ts`, `string.ts`, `css.ts`, and a follow-on +null-default in `utils.parser/src/preparse.ts` that a tightened return type +surfaces. + +- Remove `useSymbols` and `functionSupportsLengthOverwrite` internal probe + exports. Neither has external consumers in the monorepo. +- `createSymbolOrString(id) -> Symbol(id)`. +- `overwriteLengthPropertyIfSupported` always applies. +- `stringTrim` delegates to `String.prototype.trim`. +- `stringStartsWith` delegates to `String.prototype.startsWith`. +- `toggleDomNodeCssClass` uses only `node.classList`. + +Already implemented and green on `modernize/utils-dead-polyfills` — +branch: `f348e975 Drop dead polyfill probes from @tko/utils`. + +### Phase B — tasks.ts microtask scheduler + +File: `tasks.ts`. + +- `queueMicrotask` is available in every supported runtime; drop the + MutationObserver branch and the `setTimeout` final fallback. +- Collapse the scheduler selection block. +- Keep `schedule`, `cancel`, `runEarly`, `resetForTesting` exports intact. +- Validate against the MutationObserver-specific tests (if any) — `processTasks` + recursion guard stays untouched. + +### Phase C — object.ts + +File: `object.ts`. + +- Swap `Object.prototype.hasOwnProperty.call(obj, prop)` for `Object.hasOwn`. + Public `hasOwnProperty` export keeps its name. +- Delete `clonePlainObjectDeep` (already `@deprecated Function is unused`). + Verify no monorepo consumer first. +- Consider whether `extend` should simply delegate to `Object.assign` (keeps + existing `hasOwn`-filtering semantics if any consumer relies on it — check + first). + +### Phase D — memoization.ts + +File: `memoization.ts`. + +- Replace `const memos = {}` dictionary with `new Map()`. +- Replace `callback.apply(null, params || [])` with `callback(...params ?? [])`. +- Replace `new Array()` with `[]`. +- `generateRandomId` could adopt `crypto.randomUUID().slice(0,16)` but that's a + values-change — skip for this plan. + +### Phase E — array.ts idioms + +File: `array.ts`. + +- Remove `arguments.length > 2 ? action.bind(owner) : action` pattern; native + `forEach`/`map`/`filter` accept `thisArg` directly. Public export signatures + preserved (drop the third "owner" parameter? — API question, discuss in PR). +- Standardise `[]` over `new Array()`. +- Keep `compareArrays` / `findMovesInArrayComparison` — Levenshtein hot path, + don't touch behaviour. Only cosmetic cleanup. + +## Done signal + +- All 2698 browser tests (chromium, firefox, webkit) pass. +- All 2671 happy-dom tests pass. +- `bunx tsc --noEmit` clean. +- `bunx @biomejs/biome check .` clean. +- Net line count decreases; public exports unchanged. + +## Non-goals + +- No changes to `dom/*` beyond what Phase A already covers in `css.ts`. +- No changes to `options.ts` — already well-typed and self-contained. +- No new utilities or abstractions. "Don't add features, refactor, or introduce + abstractions beyond what the task requires" (AGENTS.md). From 5bec2981b9dab51d67e3ce6caedaa922de893498 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 11:08:50 -0400 Subject: [PATCH 2/2] =?UTF-8?q?plan:=20deferError=20is=20not=20a=20bug=20?= =?UTF-8?q?=E2=80=94=20false=20positive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Existing onErrorBehaviors.ts spec explicitly codifies the current behavior: when options.onError is installed, all TKO errors (including deferred ones) flow through it rather than escaping to window.onerror. That's the designed contract; my earlier flag was wrong. Co-Authored-By: Claude Opus 4.7 (1M context) --- plans/modernize-utils.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plans/modernize-utils.md b/plans/modernize-utils.md index 190fb1105..6c290e198 100644 --- a/plans/modernize-utils.md +++ b/plans/modernize-utils.md @@ -13,7 +13,7 @@ leaf package. Public API surface is preserved per phase. | C. object.ts | Not started | — | Adopt `Object.hasOwn` (ES2022); delete `@deprecated clonePlainObjectDeep`. | | D. memoization.ts | Not started | — | Replace prototype-exposed `{}` store with `Map`. | | E. array.ts idioms | Not started | — | Kill `arguments.length > 2 ? bind(thisArg)` pattern; inline native methods; standardise `[]` over `new Array()`. Public export names preserved. | -| F. (out-of-band) deferError bug | Spawned task | — | `deferError` routes through `safeSetTimeout`, so `options.onError` swallows errors meant to be surfaced. Fixed on a separate branch, not part of this plan. | +| F. ~~deferError bug~~ | Investigated — **not a bug** | — | Initial read claimed `deferError` routed through `options.onError` incorrectly. Existing spec `packages/utils/spec/onErrorBehaviors.ts` codifies this as intentional: when `onError` is installed, all TKO errors (deferred ones included) flow through it and do not re-escape to `window.onerror`. False positive. | ## Context