refactor(fjl): #32-type-system-cleanup remove deprecated data/list generics and the Slice type - #134
Open
elycruz wants to merge 3 commits into
Open
refactor(fjl): #32-type-system-cleanup remove deprecated data/list generics and the Slice type#134elycruz wants to merge 3 commits into
Slice type#134elycruz wants to merge 3 commits into
Conversation
…nerics
Removes the `@deprecated` generics from `types/data.ts` and `types/list.ts`,
migrating every call site to the replacement named in each doc block:
- `ForEachOp` -> `Ternary` (no call sites)
- `MapOp` -> `Ternary` (`object/mapObj`)
- `ReduceOp` -> `Quaternary` (`list/foldl1`, `list/foldr1`,
`list/utils/reduce{,Right,Until,UntilRight}`)
- `PredForSlice` -> `TernaryPred` (tests only)
- `SliceConstructor` -> direct type constructors (no call sites)
- `Lengthable` -> `NumberIndexable` (no call sites)
- `Nameable` -> own/native types (`{readonly name: string}`) in tests
- `ArrayType` -> constructor array types directly (no call sites)
Also renames `MapAccumOp`'s generics to the all-named convention recorded in
`types/README.md` (`AccumT`, `ElementT`, `MappedT`, `IndexT`, `ElementsT`),
removing the stray positional `B`, and keeps a default on every one of them.
Contributes to #116.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`Slice` was a hand-rolled interface standing in for exactly one thing: the built-in `string | T[]` union (its own doc block said so, and explicitly ruled out typed arrays). It also leaked `any` through `[index: number]: any` and `at(i): any`, erasing the element type it was parameterised on. It is removed with no 1:1 successor; each call site now names what it actually needs: - `NumberIndexable<T>` where only `length`/numeric indexing is read - both downstream consumers (`fjl-inputfilter/input`, `fjl-validator/lengthValidator`) and the `NumberIndexable`-based list utils' tests. - `string | T[]` where `slice`/`concat`/`at` are actually invoked - kept as a generic constraint (`TS extends string | T[]`) so "same container in, same container out" is preserved. `Iterable<T>` (the #43 API-level replacement) is not usable at these sites: the majority of them call `slice`, `concat`, `at`, or index numerically. Removes `tests/list/test-slice-type.ts`, which existed solely to assert that the `Slice` type compiled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the "keep generic defaults on method types" convention to `types/README.md` and backfills the defaults that were missing from method types outside `arity.ts`: `DefinePropertyFunc`, `DefinePropertiesFunc`, `OrderingFunc`, `ScanrOp`, and `TuplizeOp`. Replaces the README's "Deprecations" section with a "Removed types" table covering everything dropped in this change, including the `Slice` decision and when to reach for `NumberIndexable<T>` vs. `string | T[]`. Closes #113. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Summary
Work unit:
32-type-system-cleanup(epic #32, v2.0 — Type system cleanup).Removes every
@deprecatedgeneric frompackages/fjl/src/types/data.tsandpackages/fjl/src/types/list.ts, migrating all call sites to the replacement namedin each doc block; removes the leftover
Slicetype (which carried no deprecationmarker and no named replacement, so it needed a decision — see below); renames
MapAccumOp's stray positional generic; and records the "keep generic defaults onmethod types" convention in
packages/fjl/src/types/README.md.Closes #114
Closes #113
Contributes to #116
Contributes to #32
Three commits, one concern each:
d34edf82— remove the deprecateddata/listgenerics + renameMapAccumOp's generics350eb32e— removeSliceae02796f— record the conventions, backfill missing generic defaultsThe
Slicereplacement decisionDecision:
Sliceis removed with no 1:1 successor. Each call site now names whatit actually needs —
NumberIndexable<T>where onlylength/numeric indexing isread,
string | T[]whereslice/concat/atare actually invoked.Justification:
Slicewas never an abstraction over anything. Its own doc block said itmodelled "the intersection of string, array, and/or (compatible) array-like types"
and explicitly ruled out typed arrays. That is precisely the native union
string | T[], hand-rolled as an interface.any, which is what v2.0 - Type system cleanup (epic) #32 exists to fix.[index: number]: anyandat(i): anyerased the element typeTit was parameterised on —Slice<string>[0]was
any.string | T[]keeps the element type, andNumberIndexable<T>is alreadythe library's designated replacement for
Lengthable.Iterable<T>is not usable at these sites. next: ReplaceSlicetype withIterabletype #43 replacedSlicewithIterableat the iteration API surface, and that remains correct there. But 15 of the 21
internal call sites call
.slice,.concat,.at, or index numerically, none ofwhich
Iterableprovides — a naive swap does not typecheck.ArrayLike<T>would be a third spelling of a type we already export. It isNumberIndexableminus the readonly/string-friendly branch; adding it would growthe surface for nothing.
Where the "same container type in, same container type out" property mattered
(
sliceCopy,sliceFrom,sliceTo,tails,inits,subsequences,span,breakOnList,dropWhileEnd,groupBy,insertBy,cycle,last), it is preservedby keeping a generic constraint —
TS extends string | T[]— with anas TSon thereturn where TSC cannot prove
(string | T[]).slice()narrows back toTS. Those castsare the honest cost of dropping an interface that faked the same guarantee via
this.Per-type migration table
ForEachOpTernaryMapOpTernaryobject/mapObjReduceOpQuaternarylist/foldl1,list/foldr1,list/utils/reduce,reduceRight,reduceUntil,reduceUntilRightPredForSliceTernaryPredSliceConstructorStringConstructor/ArrayConstructordirectlyLengthableNumberIndexableNameable{readonly name: string})fjl/tests/object/index_test,fjl-labs/data/maybe_testArrayType†ArrayTypeConstructor/ array constructor types directlySliceNumberIndexable<T>orstring | T[](per site)_platform/slice; 17list/modules; 3list/utils/modules;fjl-inputfilter/src/input.ts;fjl-validator/src/lengthValidator.ts; 25 test files†
ArrayTypewas not on the ticket's table, but it lives intypes/data.ts, was@deprecatedwith a named replacement, and had zero call sites — removing it isexactly what #114's title asks for. Flagging it explicitly in case you'd rather keep it.
Both downstream consumers only ever read
.length, so both tookNumberIndexable<T>:packages/fjl-inputfilter/src/input.ts—!(value as Slice).lengthpackages/fjl-validator/src/lengthValidator.ts—LenValidatorOptions,$lengthValidatorNoNormalize,lengthValidator#116 (my half)
MapAccumOphad four named generics plus a stray positionalB. BecauseMapOfBandSliceOfBswere named in terms ofB, renamingBalone would have left danglingreferences, so the type adopts the all-named target shape already documented in
types/README.md:UnfoldrOp's in-code@todo(list/unfoldr.ts) is not in this PR — it belongs tothe
121-generatorswork unit.list/unfoldr.tsandlist/replicate.tsare untouched.The
types/arity.tspositional-generic exemption (Quinary,Quaternary, …) isrespected — none of their generics were renamed.
#113 (keep generic defaults)
This is a "keep/affirm" ticket, so the work was verification plus recording:
42467e2calready landed defaults onScanlOpandZipWith3Op; nothingthere was redone or undone.
MapAccumOpcarries a default on all five — including the "generic parameterised by another
declared generic" case the ticket calls out (
ElementsT ... = ElementT[]).arity.ts:DefinePropertyFunc,DefinePropertiesFunc,OrderingFunc,ScanrOp,TuplizeOp.packages/fjl/src/types/README.mdstating theconvention and its rationale, so it survives as documentation rather than as a closed
issue.
#32 module completion
I believe this legitimately completes:
list/utils/— sources, tests, and docs are free of the removed types;OrderingFuncnow carries a default.list/— with one caveat: everylist/module is migrated exceptunfoldr.ts, whoseUnfoldrOp@todois owned by the121-generatorsunit. Ticklist/once that lands.Not complete, and not claimed:
_platform/—_platform/slice/is done, but_platform/object/index.ts(and theObjectStaticsinterface behind it) still takes and returnsany. That's a separate redesign.errorThrowing/— untouched by this work unit.I have not edited issue #32.
Breaking changes
This removes exported types from published packages (
fjl, and transitively thetype surface used by
fjl-validator/fjl-inputfilter). Anything importing thesenames from
fjlwill fail to compile:SliceForEachOp,MapOp,ReduceOp,PredForSliceSliceConstructor,Lengthable,Nameable,ArrayTypeSignature changes visible to consumers:
sliceCopy,sliceFrom,sliceTo,inits,tails,subsequences,concatare nowgeneric over
TS extends string | any[]instead of takingSlice.group,intercalate,remove,removeBynow take/returnstring | T[]instead ofSlice<T>.groupBy/$groupBy,cycle,lastchanged their second generic's default fromSlice<T>toT[]._platform/slice'sat/$atare now generic over the element type.fjl-validator'sLenValidatorOptions<T>extendsValidatorOptions<NumberIndexable<T>>instead of
ValidatorOptions<Slice<T>>.Runtime behaviour is unchanged throughout — this is a types-only change.
Consumers should migrate per the table above; the same guidance is now in
packages/fjl/src/types/README.mdunder "Removed types".Testing
Baseline on
origin/mainwas 133 suites / 1066 tests. The delta is exactly one file:packages/fjl/tests/list/test-slice-type.ts(3 tests) was removed. That suite existedsolely to assert that the
Slicetype compiled — its own header reads "General ephemeraltests for
Slicetype - Tests just ensure that TSC doesn't throw any errors when using theSlicetype." With the type gone the file has nothing left to assert. No other test wasdeleted, skipped, or weakened.
Type-level verification (this ticket is entirely about types, so a green jest run isn't
sufficient evidence on its own):
* modulo the pre-existing
packages/fjl/dist/esm/object/setTheory.d.tsTS1110 errors,which reproduce identically on
origin/mainand are not from this change.Build:
rollup --config rollup.config.mjsexits 0, no warnings, no new.d.tswarnings.Lint (not a gate):
origin/main= 62 problems / 0 errors; this branch = 59 problems /0 errors. No new errors, three fewer warnings.
No git hooks were bypassed —
commit-msg,pre-commit(lint-staged), andpre-pushall ran and passed on every commit.
🤖 Generated with Claude Code