ReviewOS

also looking at this

stacks/stx

fix(loops): render the row a @break or @continue stops in

#2102
Open mkantautas wants to merge fix/foreach-break-continue-row into main
2 files +89 -43
8 days ago

Problem

In @foreach, the row that a @break or @continue stops in loses its loop variable. With the example from the Blade docs:

@foreach(users as user)
  
  <li></li>
  
@endforeach

the fifth user's row renders as <li></li>. processLoops appended the content before the marker to the output and left the iteration right away, so that content skipped the rest of the row's steps: its , @if blocks (for @break(...)/@continue(...)), :prop bindings and @include data never saw the row's context. The page pass evaluated them later without the loop variable, which gives an empty value, or a page variable of the same name. @for already keeps that content: @for(let i = 0; i < 4; i++)<li></li>@break(i === 1)@endfor renders <li>0</li><li>1</li>, while the same in @foreach rendered <li>0</li><li></li>.

Fix

The row is cut at its first @break/@continue marker, and the cut row goes through the remaining steps with the row's context before the loop stops or moves on. A marker revealed by an @if (step 4) wins over a @break(...)/@continue(...) found later in the row. When a row has both a true @break(...) and a true @continue(...), the first one in the row now decides; before, @continue always won and the <!--__STX_BREAK__--> marker was left in the HTML.

Nested @for/@while inside @foreach are still affected by an existing processing-order issue: the row's are evaluated before the nested loop runs, so their values render empty in every row, on main as well. The stopping row used to skip that step by accident, so a nested @for value that rendered there before now renders empty like in the other rows. That needs its own fix.

Tests

Seven cases in test/directives/loops.test.ts under "content before @break and @continue". On main all seven fail, for example:

Expected: "<li>a</li><li>b</li>"
Received: "<li>a</li><li></li>"

With the change they pass. bun run build, bun run smoke:install, bun run typecheck, bun run lint and bun run format:check pass, and bun test packages examples --max-concurrency=1 gives the same results as main apart from the seven new passing tests.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.