Skip to content

fix: pre-release correctness fixes for array, encoding, copy and polyfill helpers - #642

Open
nev21 wants to merge 2 commits into
mainfrom
nev21/Perf
Open

nev21 wants to merge 2 commits into
mainfrom
nev21/Perf

Conversation

@nev21

@nev21 nev21 commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Bug fixes

  • arrUnique / arrUnion: objects, arrays, functions and dates are now compared by identity (as documented) rather than by their string form, which discarded distinct values (arrUnique([{ a: 1 }, { b: 2 }]) returned [{ a: 1 }]); a Symbol no longer throws. Uses a new internal identity map (native Map when available, otherwise an array-based fallback)
  • encodeAsBase64 / decodeBase64 (and the URL variants) now use the native btoa / atob in browsers; they were called with the wrong this so every call threw and fell back to the polyfill, which differed from Node (decodeBase64("QQ") returned "A\0\0", wrapped values were corrupted)
  • encodeAsHex / encodeAsBase64 / encodeAsBase64Url no longer corrupt text containing characters above U+00FF
    • Output change: only for text that was previously corrupted, this is now encoded as the UTF-8 BOM followed by the UTF-8 bytes, eg. encodeAsHex("€") -> "efbbbfe282ac" (was "ac"); all other text is unchanged. The decode functions treat a value as UTF-8 only when it starts with the BOM and is valid UTF-8
  • objCopyProps / objExtend: the prototype pollution guard now checks the target (it checked the source), so built-in prototypes can't be changed; docs now state that only own enumerable properties are copied and that proto / constructor / prototype are skipped
  • objDeepCopy: handlers receive the correct path for an empty string key and for a key of 0 passed to details.copy()
  • arrForEach no longer throws for a string (the "in" check uses the String wrapper, created once per call and only for strings), which also fixes arrChunk, arrCompact, arrUnique, arrGroupBy, arrPartition, arrDropWhile, arrTakeWhile, arrIntersection, arrDifference and arrFlatten; arrShuffle and polyArrWith also support a string
  • polyArrAt / polyArrWith / polyArrFill convert their indexes like the native functions (+x || 0, range check, then |0), so fractional, NaN, undefined and numeric string indexes no longer return undefined, fill nothing, or add fractional properties to the array; polyArrWith([], -0.5) now throws like native. polyStrAt relies on polyArrAt for this
  • polyArrFindLastIndex / polyArrFindLast now visit every index like the native functions (and as documented); empty slots of a sparse array were skipped and a string threw a TypeError
  • iterTake closes the source iterator on the next() after the last value (like Iterator.prototype.take), not before the last value is consumed
  • arrIntersection / arrUnion no longer throw for a null or undefined array after the first (treated as empty / ignored)
  • scheduleMicrotask / scheduleNextTick no longer report enabled for a task a custom scheduleFn rejected or ran synchronously

Performance

  • objCopyProps, objExtend, deepExtend and objDeepCopy skip creating a copy context for primitive values when there is no handler (~80% faster for objects of primitives)
  • objDeepCopy / objCopyProps track visited objects with a Map when available, removing the quadratic slow down (16,000 objects: ~585ms -> ~48ms)
  • encodeAsBase64 / decodeBase64 are ~10x faster in browsers now that the native functions no longer throw on every call
  • polyArrFill no longer calls mathMin / mathMax (~17% faster)

Tests and size

  • New tests for unique, the identity map (with and without Map), string input to the array helpers, and comparisons against the native at / with / fill across ~45 kinds of index value, plus additions for encode, copy, union, intersection, iterTake, microtasks and findLastIndex
  • Size limits raised slightly for the encoding and identity map additions (es5-full 40.14 kB of 40.5 kB)

…fill helpers

Bug fixes
- arrUnique / arrUnion: objects, arrays, functions and dates are now compared by identity (as documented) rather than by their string form, which discarded distinct values (arrUnique([{ a: 1 }, { b: 2 }]) returned [{ a: 1 }]); a Symbol no longer throws. Uses a new internal identity map (native Map when available, otherwise an array-based fallback)
- encodeAsBase64 / decodeBase64 (and the URL variants) now use the native btoa / atob in browsers; they were called with the wrong `this` so every call threw and fell back to the polyfill, which differed from Node (decodeBase64("QQ") returned "A\0\0", wrapped values were corrupted)
- encodeAsHex / encodeAsBase64 / encodeAsBase64Url no longer corrupt text containing characters above U+00FF
  - Output change: only for text that was previously corrupted, this is now encoded as the UTF-8 BOM followed by the UTF-8 bytes, eg. encodeAsHex("€") -> "efbbbfe282ac" (was "ac"); all other text is unchanged. The decode functions treat a value as UTF-8 only when it starts with the BOM and is valid UTF-8
- objCopyProps / objExtend: the prototype pollution guard now checks the target (it checked the source), so built-in prototypes can't be changed; docs now state that only own enumerable properties are copied and that __proto__ / constructor / prototype are skipped
- objDeepCopy: handlers receive the correct path for an empty string key and for a key of 0 passed to details.copy()
- arrForEach no longer throws for a string (the "in" check uses the String wrapper, created once per call and only for strings), which also fixes arrChunk, arrCompact, arrUnique, arrGroupBy, arrPartition, arrDropWhile, arrTakeWhile, arrIntersection, arrDifference and arrFlatten; arrShuffle and polyArrWith also support a string
- polyArrAt / polyArrWith / polyArrFill convert their indexes like the native functions (+x || 0, range check, then |0), so fractional, NaN, undefined and numeric string indexes no longer return undefined, fill nothing, or add fractional properties to the array; polyArrWith([], -0.5) now throws like native. polyStrAt relies on polyArrAt for this
- polyArrFindLastIndex / polyArrFindLast now visit every index like the native functions (and as documented); empty slots of a sparse array were skipped and a string threw a TypeError
- iterTake closes the source iterator on the next() after the last value (like Iterator.prototype.take), not before the last value is consumed
- arrIntersection / arrUnion no longer throw for a null or undefined array after the first (treated as empty / ignored)
- scheduleMicrotask / scheduleNextTick no longer report `enabled` for a task a custom scheduleFn rejected or ran synchronously

Performance
- objCopyProps, objExtend, deepExtend and objDeepCopy skip creating a copy context for primitive values when there is no handler (~80% faster for objects of primitives)
- objDeepCopy / objCopyProps track visited objects with a Map when available, removing the quadratic slow down (16,000 objects: ~585ms -> ~48ms)
- encodeAsBase64 / decodeBase64 are ~10x faster in browsers now that the native functions no longer throw on every call
- polyArrFill no longer calls mathMin / mathMax (~17% faster)

Tests and size
- New tests for unique, the identity map (with and without Map), string input to the array helpers, and comparisons against the native at / with / fill across ~45 kinds of index value, plus additions for encode, copy, union, intersection, iterTake, microtasks and findLastIndex
- Size limits raised slightly for the encoding and identity map additions (es5-full 40.14 kB of 40.5 kB)
@nev21 nev21 added this to the 0.20.0 milestone Nov 11, 2026
@nev21
nev21 requested review from a team as code owners November 11, 2026 06:25
Copilot AI balanced review requested due to automatic review settings November 11, 2026 06:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@codecov

codecov Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.45946% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.23%. Comparing base (f3a3c14) to head (3a6ef03).

Files with missing lines Patch % Lines
lib/src/helpers/encode.ts 98.18% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #642   +/-   ##
=======================================
  Coverage   99.22%   99.23%           
=======================================
  Files         181      182    +1     
  Lines        5688     5726   +38     
  Branches     1323     1340   +17     
=======================================
+ Hits         5644     5682   +38     
  Misses         44       44           
Files with missing lines Coverage Δ
lib/src/array/at.ts 100.00% <100.00%> (ø)
lib/src/array/drop.ts 100.00% <100.00%> (ø)
lib/src/array/drop_while.ts 100.00% <100.00%> (ø)
lib/src/array/fill.ts 100.00% <100.00%> (ø)
lib/src/array/forEach.ts 100.00% <100.00%> (ø)
lib/src/array/intersection.ts 100.00% <100.00%> (ø)
lib/src/array/rotate.ts 100.00% <100.00%> (ø)
lib/src/array/shuffle.ts 100.00% <100.00%> (ø)
lib/src/array/slice.ts 100.00% <100.00%> (ø)
lib/src/array/take.ts 100.00% <100.00%> (ø)
... and 11 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Several active moderate issues remain in array index handling and string support, including 32-bit wraparound that targets incorrect positions and a string path that returns the wrong result type.

4 open issues

Review effort: Lite

Comment thread lib/src/array/at.ts Resolved
Comment thread lib/src/array/fill.ts Resolved
Comment thread lib/src/array/forEach.ts
Comment on lines +59 to +62
// The "in" operator throws for a string primitive (which is ArrayLike), so use its object form
const theObj: any = isString(theArray) ? ObjClass(theArray) : theArray;
for (let idx = 0; idx < len; idx++) {
if (idx in theArray) {
if (idx in theObj) {
Comment thread lib/src/array/with.ts Resolved
nevware21-bot
nevware21-bot previously approved these changes Nov 11, 2026

@nevware21-bot nevware21-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by nevware21-bot

Follow-up to the pre-release fixes: resolves the Copilot review comments on
#642 and wins back most of the bundle size they added
(es5-full 40.14 kB -> 39.55 kB, gzip 15.05 kB -> 14.77 kB).

Review fixes
- polyArrAt / polyArrWith / polyArrFill now truncate the index with `idx -= idx % 1` rather than `idx |= 0`. The bitwise version wrapped indexes above 2^31 - 1 (array-likes and sparse arrays can be longer), so at() read and fill() wrote the wrong position, and an unbounded fill could make ~2^31 writes
- arrDropWhile, arrDrop, arrTake and arrRotate (when there is no rotation) now return an array for a string, as documented, rather than a string. The new internal _arrSliceToArr uses Array.prototype.slice for a string and arrSlice otherwise (so a value's own slice, eg. a typed array's, is still used), and also replaces the duplicated logic in polyArrWith and arrShuffle

Size reductions
- encode: UTF-8 conversion now uses encodeURIComponent / decodeURIComponent rather than a hand-written encoder, decoder and two large regexes (and without the deprecated escape / unescape). As encodeURIComponent rejects an unpaired surrogate, it is now encoded as U+FFFD like TextEncoder a Buffer (previously it round tripped). The native btoa / atob paths share one _createBase64Fn (looked up once via getInst, called without a `this` and falling back to the polyfill on error), replacing the two safeGetDeferred blocks; consecutive push calls are merged
- The internal identity map is now a single "get or add" function, which is all its callers need; the array fallback for runtimes without Map keeps replacing existing keys and also handles NaN
- arrUnique uses the identity map for every value rather than building astring key for primitives, ~5x faster for numbers and strings (runtimeswithout Map, IE 9/10, use a linear search)
- objDeepCopy / objCopyProps: removed the unused visit entry `k` and`newPath` parameter. _deepCopy identifies the value before theno-handler fast path, so each value needs at most one helper call(isPlainObject for an object, isPrimitiveType otherwise)

Performance
- arrUnique ~5-6x faster for primitives and ~12% faster for objects;objExtend ~4% faster; other functions within noise
- Encoding text above U+00FF is slower than the hand-written version(encode ~0.8 -> 2.4 us, decode ~0.9 -> 2.1 us for a short string); ASCII and Latin-1 text never reaches this code

Tests
- Indexes above 2^31 for polyArrAt, polyArrWith and polyArrFill
- String input for arrDropWhile, arrTakeWhile, arrDrop, arrTake and arrRotate
- Unpaired surrogates and text containing "%" in the encoding round trips
- The identity map's get-or-add behaviour, with and without a native Map
Copilot AI lite review requested due to automatic review settings November 11, 2026 08:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

A moderate active finding remains in lib/src/array/rotate.ts: non-zero string rotations can produce whole string segments instead of individual characters on ES5 runtimes.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Convert rotation slices to arrays for strings

lib/​src/​array/​rotate.ts:9

The non-zero rotation branch still uses arrSlice for the two pieces below. For a string, arrSlice resolves String.prototype.slice and returns strings; on ES5 runtimes where strings are not iterable, arrAppend adds each piece as one element, so arrRotate("abc", 1) becomes ["bc", "a"] instead of ['b', 'c', 'a']. Use _arrSliceToArr for all slices (for example, alias it as arrSlice in this import).

3 resolved since last review

Review effort: Lite

@nevware21-bot nevware21-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by nevware21-bot

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants