Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterate more node lists with foreach #91

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Mar 18, 2024

PHP follows the DOM specification, which requires that NodeList objects in the DOM are live. As a result, any operation that removes a NodeList node from its parent (e.g. removeChild, replaceChild or appendChild) will cause the next node in the iterator to be skipped.

Let’s convert those node lists to arrays to make them static to allow us to use foreach and drop the index decrementing.

This will make the code a bit clearer at the cost of slightly more work being performed.

Follow-up to #90

`DOMNode::$childNodes` always contained `DOMNodeList`.
This simplifies the code a bit and will make it slightly easier in case we decide to switch to `foreach` iteration.
This was forgotten in b580cf2.
`DOMNodeList` implements `Traversable`.

There are some `for` loops left but we cannot simply replace those:
PHP follows the DOM specification, which requires that `NodeList`
objects in the DOM are live. As a result, any operation that removes
a node list member node from its parent (such as `removeChild`,
`replaceChild` or `appendChild`) will cause the next node
in the iterator to be skipped.

We could work around that by converting those node lists to static arrays
using `iterator_to_array` but not sure if it is worth it.
PHP follows the DOM specification, which requires that NodeList objects
in the DOM are live. As a result, any operation that removes a NodeList
node from its parent (e.g. removeChild, replaceChild or appendChild)
will cause the next node in the iterator to be skipped.

Let’s convert those node lists to arrays to make them static to allow
us to use foreach and drop the index decrementing.

This will make the code a bit clearer
at the cost of slightly more work being performed.
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.

1 participant