skiptools / skiptools/skip-ui

ForEach.produceLazyItems discards all but the first renderable, silently and only on the lazy path

Open
#509 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Swift
Stars
330
Forks
76
Avg merge
3h 6m
Merged PRs (30d)
1

Description

ForEach.produceLazyItems discards all but the first renderable — silently, and only on the lazy path

Small one, and possibly intended — but it is undocumented and it currently makes a rendering bug into a diagnostic. Line references are against main @ c8332cb.

What happens

produceLazyItems keeps only the first renderable per element (Containers/ForEach.swift:209, :222, :232):

let renderable = renderables.firstOrNull() ?? EmptyView()

The unroll decision that guards this path is taken from the first element only (isUnrollRequired, ForEach.swift:195-203, guard isFirst else { return true }). So a body that yields one top-level view for element 0 and more than one for a later element takes the lazy path — and every extra view from element 1 onward is dropped with no log and no error. grep for logging across ForEach.swift and LazySupport.swift returns nothing.

A shape that does this, where the separator is conditional on something other than "not the first row":

LazyVStack {
    ForEach(lines) { line in
        row(for: line)
        if line.needsSeparator {      // false for lines[0], true later
            Divider()
        }
    }
}

Why it matters to us

We hit the mirror image of it. Our body was row(for:) plus if line.id != lines.last?.id { Divider() }, which yields two views for element 0 of any list of two or more — so we unrolled, and the dividers rendered. That gave us a free diagnostic we now rely on internally: for a sibling-divider shape, visible dividers mean the ForEach unrolled, because the lazy path would have eaten them.

Depending on a rendering bug to tell you which code path you are on is a strange place to end up, which is why we are raising it even though it never bit us directly.

Ask

Either is fine, and we do not have a strong view on which:

  • If the discard is intended (a lazy item slot cannot emit two items — your own comment at ForEach.swift:86-90), a sentence saying so next to the existing unroll note at README.md:2681 would close it. That note is accurate and we should have found it sooner; it just does not cover what happens to the extras when the unroll decision has already gone the other way.
  • If it is not intended, :209 / :222 / :232 are the place, and unrolling instead of discarding when renderables.size > 1 would seem to preserve the existing guarantee.

Sibling issue about non-LazyItemFactory children collapsing a subtree into one lazy item: #508 — that one is where our actual latency went.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in Containers/ForEach.swift at lines 195-232 and read the lazy-item constraint comment at lines 86-90. Compare the existing unroll note at README.md:2681 and issue #508, then establish whether extra renderables should be documented as discarded or should trigger unrolling. Done means the chosen behavior is implemented or clearly documented without relying on the rendering bug as a diagnostic.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.