fix(nanoviews): show the for_ placeholder when there is no array at all - #205
Merged
Conversation
… all A reactive `for_` read `items.length` and threw on a nullish array - both on the first render and on a change from a list to nothing. Only the static form handled it, because it happened to be written with `?.`. No array is the same emptiness as an empty one: the rows go and the placeholder comes. The nullable array is a separate overload arm rather than a widening of the writable one. A signal is invariant, so `WritableSignal<T[]>` does not fit `WritableSignal<T[] | EmptyValue>`, and widening in place would take the rows of every existing caller down to read-only.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #205 +/- ##
=======================================
Coverage 85.32% 85.32%
=======================================
Files 139 139
Lines 3142 3142
Branches 591 591
=======================================
Hits 2681 2681
Misses 332 332
Partials 129 129 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
for_over a signal that holdsnullorundefinedthrew:Both on the first render and on a change from a list to nothing. The static form already did the right thing — it happens to be written
$items?.length ? … : else_?.()— so the two forms disagreed about the same input.No array is the same emptiness as an empty one, so the fix is to say that:
Nothing else in the loop needed a guard. The array is only touched further down when
itemsCountis truthy, and TypeScript narrows it from that alone — two attempts to add a!were rejected by the linter as unnecessary assertions.The overload arm
The interesting half is the types. Widening the writable overload from
WritableSignal<T[]>toWritableSignal<T[] | EmptyValue>looks like the obvious move and is wrong: a signal is invariant, soWritableSignal<Player[]>does not fit the widened parameter, every existing caller falls through to the readable overload, and its rows silently become read-only. Four existing tests failed withExpected 0 arguments, but got 1on a row write.So the nullable array is a separate arm of the same signature:
The readable and static overloads widen normally.
Tests
One test walking all four transitions: absent on first render shows the placeholder, absent → list shows the row, list → absent brings the placeholder back, and absent →
[]leaves it alone rather than tearing it down and building it again. 110 pass.Size: +1 B gzip on all publics, average usage unchanged, every pin stays where it is.