Skip to content

fix: preserve a trailing hole in sparse arrays#46

Open
spokodev wants to merge 1 commit into
blakeembrey:mainfrom
spokodev:fix/sparse-array-trailing-hole
Open

fix: preserve a trailing hole in sparse arrays#46
spokodev wants to merge 1 commit into
blakeembrey:mainfrom
spokodev:fix/sparse-array-trailing-hole

Conversation

@spokodev

Copy link
Copy Markdown

Problem

A sparse array whose last element is a hole loses that hole on a round-trip, so the array comes back shorter:

import { stringify } from "javascript-stringify";

const value = [1, , ]; // length 2 (a hole at index 1)
stringify(value);       // "[1,]"
eval("(" + stringify(value) + ")").length; // 1  ← should be 2

Leading and interior holes are fine ([,1], [1,,3]); only a trailing hole is dropped.

Root cause

arrayToString maps the values (Array.prototype.map preserves holes) and joins them:

const values = array.map(...).join(",");
return `[${eol}${values}${eol}]`;

join renders a hole as an empty string between separators, which works for leading and interior holes. But a trailing hole produces a single trailing comma — and [1,] is a length-1 array literal, so the hole is lost.

Fix

When the last element is a hole, append one more separator so the array literal keeps its length ([1,,] has length 2):

const trailingHole =
  array.length > 0 && !(array.length - 1 in array) ? "," : "";
return `[${eol}${values}${trailingHole}${eol}]`;

Test plan

  • Added round-trip tests for trailing, leading and interior holes (the two trailing-hole cases fail on main, the others already passed).
  • Full suite green (150).

`Array.prototype.map` keeps holes, but joining the results drops a
trailing one: `[1, ,]` (length 2) was serialized as `[1,]`, which is an
array of length 1. So a sparse array ending in a hole came back shorter
after `eval`. Append a separator when the last element is a hole to keep
the length.
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.

1 participant