Skip to content

Commit 427a37c

Browse files
guitavanoclaude
andauthored
fix(engine): drop never-gated hidden array items instead of null holes (#1217)
* fix(engine): drop never-gated hidden array items instead of null holes Hiding an array item wraps it in a multivariate flag gated by a `never` matcher; flag.ts returns `undefined` when no variant matched. The engine left that `undefined` as a hole in the resolved array, which serializes to `null` in JSON and renders as an empty card (blank benefit rows, etc.). Compact `undefined` out of resolved arrays in resolvePropsWithHints. A resolver returning `null` is a legitimate value and is kept — only the `undefined` "not present" sentinel is filtered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(engine): gate array compaction on includes + broaden tests Address review feedback: - Only reallocate when the array actually contains `undefined` (scan with `includes` first) so the common zero-undefined case skips the extra allocation on the hot resolve path. - Reframe the comment around the generic invariant (object props already drop `undefined` under JSON.stringify; array `undefined` serializes to a `null` hole — compaction makes arrays consistent), citing the never-gated flag as the motivating case rather than the definition. - Add tests: all-items-hidden -> [], and nested-array multi-depth (with a first-element drop to pin ordering). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 53410fe commit 427a37c

2 files changed

Lines changed: 94 additions & 0 deletions

File tree

engine/core/mod.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,87 @@ Deno.test("resolve", async (t) => {
153153
});
154154
});
155155

156+
await t.step(
157+
"drops hidden array items (resolver -> undefined) but keeps null",
158+
async () => {
159+
// A hidden array item is a multivariate flag gated by a `never` matcher;
160+
// flag.ts returns `undefined` when no variant matched. Such items must be
161+
// dropped from the array, not left as holes (which serialize to `null`
162+
// and render as empty cards). A resolver returning `null` is a legitimate
163+
// value and is kept.
164+
const resolverMap = {
165+
resolve: (data: unknown) => context.resolve(data),
166+
hiddenFlag: (): unknown => undefined,
167+
nullFlag: (): unknown => null,
168+
keep: (p: { label: string }) => p,
169+
};
170+
const result = await resolve<{ items: unknown[] }>(
171+
{
172+
items: [
173+
{ label: "A", __resolveType: "keep" },
174+
{ __resolveType: "hiddenFlag" },
175+
{ label: "B", __resolveType: "keep" },
176+
{ __resolveType: "nullFlag" },
177+
{ __resolveType: "hiddenFlag" },
178+
],
179+
__resolveType: "resolve",
180+
},
181+
{ ...context, resolvers: resolverMap as unknown as ResolverMap },
182+
);
183+
assertEquals(result, {
184+
items: [{ label: "A" }, { label: "B" }, null],
185+
});
186+
},
187+
);
188+
189+
await t.step(
190+
"an array where every item is hidden becomes empty (not a hole array)",
191+
async () => {
192+
const resolverMap = {
193+
resolve: (data: unknown) => context.resolve(data),
194+
hiddenFlag: (): unknown => undefined,
195+
};
196+
const result = await resolve<{ items: unknown[] }>(
197+
{
198+
items: [
199+
{ __resolveType: "hiddenFlag" },
200+
{ __resolveType: "hiddenFlag" },
201+
],
202+
__resolveType: "resolve",
203+
},
204+
{ ...context, resolvers: resolverMap as unknown as ResolverMap },
205+
);
206+
assertEquals(result, { items: [] });
207+
},
208+
);
209+
210+
await t.step(
211+
"hidden items are dropped at every array depth (nested arrays)",
212+
async () => {
213+
// The fix relies on the filter firing at each array level via recursion,
214+
// and on ordering being preserved when the first element is dropped.
215+
const resolverMap = {
216+
resolve: (data: unknown) => context.resolve(data),
217+
hiddenFlag: (): unknown => undefined,
218+
keep: (p: { label: string }) => p,
219+
};
220+
const result = await resolve<{ groups: unknown[][] }>(
221+
{
222+
groups: [
223+
[
224+
{ __resolveType: "hiddenFlag" },
225+
{ label: "A", __resolveType: "keep" },
226+
],
227+
[{ __resolveType: "hiddenFlag" }],
228+
],
229+
__resolveType: "resolve",
230+
},
231+
{ ...context, resolvers: resolverMap as unknown as ResolverMap },
232+
);
233+
assertEquals(result, { groups: [[{ label: "A" }], []] });
234+
},
235+
);
236+
156237
await t.step("resolves object with no resolvable fields", async () => {
157238
type TestType = {
158239
foo: string;

engine/core/resolver.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,19 @@ const resolvePropsWithHints = async <
424424
}
425425

426426
if (!type) {
427+
// Compact `undefined` out of resolved arrays. An array element that resolved
428+
// to `undefined` is a "not present" value (the motivating case: a hidden
429+
// item — a multivariate flag gated by a `never` matcher, which flag.ts
430+
// resolves to `undefined` when no variant matched). Object properties that
431+
// resolve to `undefined` already vanish under `JSON.stringify`, but an
432+
// `undefined` array element serializes to a `null` hole that the consuming
433+
// section renders as an empty card — so filter it, making arrays consistent
434+
// with object props. A resolver returning `null` is a legitimate value and
435+
// is kept. `includes` first so the common (nothing-to-drop) case skips the
436+
// reallocation the surrounding hot path deliberately avoids.
437+
if (Array.isArray(mutableProps) && mutableProps.includes(undefined)) {
438+
return mutableProps.filter((item) => item !== undefined) as T;
439+
}
427440
return mutableProps;
428441
}
429442

0 commit comments

Comments
 (0)