EPBDS-16211 E1: src/core/utils to strict TypeScript, 20 files - #55
Merged
Merged
Conversation
Type-only. classNames, codec, constants, lodash-lite, media, selectors,
time, validators — the files with no intra-utils dependencies.
1072 -> 1160 lines. 20 explicit `any`, 18 of them in lodash-lite, which is
a deliberately polymorphic lodash reimplementation; the rest of the wave
needed two, both in codec where the honest type is JSON.parse's own `any`.
ALSO IN THIS COMMIT, because the renames require it: 38 relative imports
inside src/core/utils lose their explicit `.js` extension. Jest and webpack
resolve `'./codec.js'` literally, so a .js -> .ts rename breaks it, and tsc
cannot warn because checkJs is false — it fails at test and build time
instead. The extension style existed in exactly two places in the repo:
these 38 specifiers, and 17 in src/demo/examples/manifest.js which point at
files that stay .js and are untouched. Against 824 extensionless relative
imports elsewhere, utils was the anomaly, not the convention.
ONE CODE MOVE, not a type change: lodash-lite's `capitalize` declaration
now sits above the `export { … }` block instead of below it. Reason: for
.js, eslint-config-react-app sets no-use-before-define to "off"; for .ts it
enables the @typescript-eslint variant at "warn", so the pre-existing
pattern started failing the zero-warning gate purely because the extension
changed. Rather than weaken the rule for TypeScript I moved the one
declaration that triggers it, and proved the move is inert: Babel's output
for the file is byte-identical before and after (12047 bytes both).
Verified: typecheck clean, lint:js clean, 26 utils suites / 565 tests green,
full suite 2530 green, build-lib green, static/all.css sha256 unchanged.
Type-only, zero `any` across all three. 719 -> 806 lines, 53 agent-minutes.
_envs needed the most thought and none of it was about syntax: Active is a
module-global that platform code attaches ad-hoc properties to at runtime
(Active.Field, Active.renderField, Active.UIRender, Active.SERVICE), so it
carries an index signature rather than a closed shape, and the env-dependent
slots (Storage, WebSocket, client, log, history) are `unknown` rather than
their DOM types — backends replace Active.Storage with an adapter that has
init()/initSync(), which the DOM Storage interface does not have. Typing it
as Storage would have been a lie that compiled.
Three call sites were identified as needing attention when THEIR file
converts, and deliberately left alone here: storage.ts will need to narrow
Active.Storage; utility.ts's `Active.passwordCheck(password).score` needs a
null check or an explicit non-null assertion, because on the backend
passwordCheck is undefined and that line throwing is the current behaviour;
and modules/variables/_envs.js assigns a passwordCheck that returns
undefined, which is a behaviour question, not a type question.
components.ts is a one-line re-export with NO callers anywhere — every real
consumer of `cn` imports classNames directly. Converted as-is; flagged for a
dead-code pass rather than deleted inside a type-only change.
EXTENSIONED SPECIFIERS, ROUND TWO. Wave 1 stripped `.js` from imports inside
src/core/utils, but only from `from '…'` forms in the modules themselves. The
sweep missed jest.doMock / jest.requireActual / require() and it missed the
__tests__ directory entirely, so converting string.js broke five tests in
utility.id-collisions-and-history.test.js, which mocks '../string.js' by
explicit path. Fixed, and the search was redone across all of src in every
call form. What it found: the only other in-src hazard is Table.test.js's
require.resolve('../Table.js'), which would have broken when E2 converts
Table — defused here while it is a one-line change rather than a red CI run.
Everything else pointing at a `.js` path is in src/style tests referencing
scripts/ and postcss.config.js, which live outside src and are not migrating.
Type-only. 650 -> 745 lines, 57 agent-minutes, 2 `any` (both in array). array's two `any` are the comparator type for `by()`. `unknown` parameters there are contravariant and would reject every caller that annotates its own comparator — `by((a: Row, b: Row) => a.value - b.value)` would stop compiling. Same reasoning lodash-lite already records for its callbacks. `toList`, `toUniqueList` and `firstListValue` became overload signatures over a loose implementation rather than gaining generics that would have needed their bodies rewritten. definitions took zero `any` but pushes work downstream, which is worth knowing before E2: definition VALUES are `unknown`, because the tests pass null, 0 and arrays as values and as `_` codes. So a future TypeScript caller of FIELD.TYPE.X or ENUM.LANGUAGE has to narrow. Typing them `string` would have been convenient and false. Two deliberate non-edits, both preserving behaviour over tidiness: definitionSetup is typed as if every prop is always present, though the getter really returns undefined until first assignment (a test asserts this). Every call site assigns at module init, and `Definition | undefined` would put optional chaining on every FIELD.TYPE.X in the codebase. localise gained an unused optional second parameter so the existing recursive `localise(definition, Active)` call typechecks WITHOUT editing that line. The only runtime-visible effect is localise.length becoming 2. Deleting the ignored argument would have been a real source edit. Verified by exit code, not by reading output: typecheck, lint:js and the full suite (168 suites / 2530 tests) all exit 0.
…ct TypeScript
Type-only. 1407 -> 1523 lines, 87 agent-minutes, 2 `any` (both in object).
object is the biggest file in the package at 688 lines and it came in at
MODERATE, not HARD — path manipulation over arbitrary shapes types more
cleanly than the polymorphic helpers did. Its agent caught and reverted its
own near-miss during the work: a guard it had briefly added inside
hasObjKeys would have been a behaviour change wearing a type change's
clothes.
number's types are honest where the JSDoc was not: formatNumber is declared
`string | number`, because a non-numeric argument is returned unchanged
(formatNumber('abc') === 'abc', formatNumber(NaN) === NaN) — the JSDoc has
always claimed `{string}`.
function keeps `arguments` rather than rest parameters in debounce so the
emitted JavaScript stays byte-identical; the cost is two `as unknown as`
casts around func.apply to get past strictBindCallApply. Ugly and inert —
both erase to `func.apply(self, args)`. isFunction is a predicate over the
broad `Function`, not a call signature: a narrower one would have made every
existing call site uncallable or wrongly-arity'd.
ONE THING TO CARRY INTO E2, flagged by the number agent rather than
papered over: src/core/components/renders.js:139 calls round(value, decimals)
where value can be a numeric string from data.json, while round is typed
`number` per its JSDoc. Nothing breaks today because renders.js is unchecked
JavaScript. Whoever converts renders.js has to decide whether to widen the
six rounding helpers or cast at that call site — and that is a real question
about what the contract is, not a mechanical one.
Verified by exit code: typecheck, lint:js and the full suite all exit 0.
…ript
storage and utility (0 `any`), then index.ts. 20 of 20 files converted.
Both wave-5 agents made the call I wanted on the two places where a type
would have needed a behaviour change to be honest, and made it the same way:
utility's `Active.passwordCheck(password).score` uses a non-null assertion,
NOT a guard. passwordCheck is undefined on the backend and that line
throwing is the current behaviour; a safe default would have been a silent
functional change.
storage casts Active.Storage to a local adapter type instead of narrowing
at runtime. _envs types that slot `unknown` because backends replace it
with their own adapter; a runtime check would have added behaviour.
storage also needed `declare namespace` merged with the function declaration
to type performStorage's expando statics, because two of them are assigned
from inside init/initSync where TypeScript's expando inference does not
reach. It is ambient, so Babel strips it entirely.
COVERAGE WAS SILENTLY LOSING THE CONVERTED FILES — found because
test:coverage went red while plain jest stayed green. `collectCoverageFrom`
listed `{js,jsx}` only, so every file dropped out of measurement the moment
it became `.ts`. Since utils is among the best-covered code in the repo,
losing it pushed the GLOBAL percentages DOWN and the thresholds red. Three
per-file thresholds also still pointed at `./src/core/utils/*.js` paths that
no longer exist, which jest reports as "coverage data not found" rather than
as an error. Both fixed. This is the same class of hole as E0's lint `--ext`:
a gate keyed on file extension stops watching whatever the migration moves,
and says nothing.
After the fix: 97.05 / 94.06 / 96.88 / 97.77 — unchanged from before E1,
which is the point. The conversion never lost coverage; the gate lost sight
of it.
Verified by exit code: typecheck, lint:js, jest, test:coverage, build-lib,
build-css, css:fixture:check and both docs checks all exit 0.
static/all.css sha256 unchanged.
The §9.6 go/no-go after E1 was written to turn on "measured E1 conversion velocity" and there was no measurement. There is one now, per wave, in the plan: 20 files, 4142 -> 4578 lines, 364 agent-minutes, 23 `any` against 199 `unknown`. Also corrected: E1 described src/core/utils as "~36 files". It is 20. The figure predates H1 and the earlier deletions. The part worth reading before deciding the gate is what the numbers do NOT support. Cost tracked polymorphism, not size — lodash-lite at 552 lines cost 55 minutes and carries 17 of the 23 `any`, while string at 663 lines cost 35 and needed none. And utils is pure and React-free, so it says nothing about the components E2 would take on. The plan now argues the go/no-go should turn on a two-or-three component pilot rather than on extrapolating this. Three infrastructure holes are recorded too, because they share one shape and will recur: a gate keyed on file extension stops watching whatever the migration moves, and says nothing while it does. Extensioned import specifiers, lint that did not see .ts until E0 extended it, and coverage collection that silently dropped every converted file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type-only.
src/core/utilsis now entirely TypeScript — 20 of 20 files, strict, suite and coverage thresholds unchanged.Six commits, one per dependency wave, each verified green on its own.
The measurement the go/no-go was written to need
§9.6's gate says the E2/E3 decision turns on "measured E1 conversion velocity". There was no measurement. There is now:
any_envs,components,stringarray,definitionsfunction,number,object,translationsstorage,utility+ barrelCost tracks polymorphism, not size.
lodash-lite— 552 lines, a lodash reimplementation — took 55 minutes and carries 17 of the 23anyalone.string, at 663 lines, took 35 minutes and needed none. Package-wide: 23anyagainst 199unknown.What this does not support: extrapolating to the ~155 remaining
.js/.jsx.utilsis pure and React-free; E2's components bring JSX, hooks, prop types and the DOM boundary. The plan now argues the gate should turn on a two-or-three component pilot instead.Three infrastructure holes, one shape
A gate keyed on file extension stops watching whatever the migration moves, and says nothing while it does.
utilsimports carried an explicit.jsextension. Jest and webpack resolve those literally, so every rename broke resolution — andtsccannot warn, becausecheckJsis off. It fails at test and build time instead. A second round found the same thing injest.doMock/requireforms and inside__tests__, which my firstfrom '…'sweep missed. DefusedTable.test.js'srequire.resolve('../Table.js')too, while it is one line rather than a red CI run in E2.eslint-config-react-appturnsno-use-before-defineOFF for.jsand ON for.ts. I moved one declaration rather than weaken the rule, and proved the move inert: Babel's output is byte-identical, 12047 bytes before and after.collectCoverageFromlisted{js,jsx}. Becauseutilsis among the best-covered code here, losing it pushed the global numbers down and the thresholds red — while plainjeststayed green. Three per-file thresholds also still named*.jspaths, which jest reports as "coverage data not found", not as an error. After the fix: 97.05 / 94.06 / 96.88 / 97.77, unchanged from before E1 — the conversion never lost coverage, the gate lost sight of it.Behaviour was held still, and where that cost something it is recorded
definitions.localisegained an unused optional parameter so an existing recursive call typechecks without editing that line.utilityuses a non-null assertion onActive.passwordCheck, not a guard — it being undefined and throwing is the backend behaviour.function.debouncekeepsargumentsrather than rest parameters so the emitted JS stays byte-identical, at the cost of two inertas unknown ascasts.objectagent caught and reverted its own near-miss: a guard it briefly added insidehasObjKeyswould have been a behaviour change wearing a type change's clothes.One real contract question, left for its owner
src/core/components/renders.js:139callsround(value, decimals)wherevaluecan be a numeric string fromdata.json, while the six rounding helpers are typednumberper their JSDoc. Nothing breaks today —renders.jsis unchecked JavaScript. Widening the helpers or casting at the call site is a decision about what the contract is, so I did not make it.Still open in E1
The contract types for meta/data (the §9.4 half — one source of truth between types and JSON Schema, plus a round-trip check). That is a design piece, not a mechanical one, and deserves its own PR.
Verified
Every gate by exit code, not by reading output:
typecheck,lint:js,jest,test:coverage,build-lib,build-css,css:fixture:checkand both docs checks exit 0.static/all.csssha256 unchanged atdcdb0a40a6d8ee79….🤖 Generated with Claude Code