Skip to content

[5.5] Set up loop variable correctly on all Traversable objects - #23388

Merged
taylorotwell merged 1 commit into
laravel:5.5from
halloei:5.5
Mar 5, 2018
Merged

[5.5] Set up loop variable correctly on all Traversable objects#23388
taylorotwell merged 1 commit into
laravel:5.5from
halloei:5.5

Conversation

@halloei

@halloei halloei commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

The $loop variable within Blade's @foreach is not filled correctly if the object given implements \Traversable but not \Countable. The reason is that the stack size is not determined properly. This leads to unexpected behaviour using i.e. $loop->last.

This commit fixes the $loop variable so it works correctly with all \Traversable stacks (like \DatePeriod). It won't break any existing features as it just extends the functionality.

if (is_array($data) || $data instanceof Countable) {
$length = count($data);
} elseif ($data instanceof Traversable) {
$length = iterator_count($data);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks when the data used in the view comes from a generator (using yield)...

Exception(code: 0): Cannot traverse an already closed generator

@halloei @taylorotwell

taylorotwell pushed a commit that referenced this pull request Mar 24, 2018
* Failing test for views looping over generators

* Revert "Set up loop variable correctly on all Traversable objects (#23388)"

This reverts commit 12dcc57.

* Style fixes
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.

3 participants