Skip to content

Allow pushing nodes to NodeList via []=#767

Merged
vladar merged 6 commits intowebonyx:masterfrom
spawnia:allow-pushing-to-node-list
Jan 23, 2021
Merged

Allow pushing nodes to NodeList via []=#767
vladar merged 6 commits intowebonyx:masterfrom
spawnia:allow-pushing-to-node-list

Conversation

@spawnia
Copy link
Collaborator

@spawnia spawnia commented Jan 20, 2021

This can come up when manipulating the AST programmatically.
When pushing to a NodeList using []=, the first argument
to offsetExists is null.

Thus, the element is set into the internal array with (string) null === "".
Now, additional pushes replace the node at key "" instead of appending as expected.

This can come up when manipulating the AST programmatically.
When pushing to a `NodeList` using `[]=`, the first argument
to `offsetExists` is `null`.

Thus, the element is set into the internal array with `(string) null === ""`.
Now, additional pushes replace the node at key `""` instead of appending as expected.
@coveralls
Copy link

coveralls commented Jan 20, 2021

Coverage Status

Coverage increased (+0.07%) to 86.254% when pulling c7b7130 on spawnia:allow-pushing-to-node-list into 5ecd260 on webonyx:master.

@spawnia
Copy link
Collaborator Author

spawnia commented Jan 20, 2021

@vladar can i merge this and cut a bugfix release? Just ran into this bug in Lighthouse.

@spawnia spawnia added the bug label Jan 20, 2021
Copy link
Member

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Nice! 👍

@vladar vladar merged commit eb58ab1 into webonyx:master Jan 23, 2021
@spawnia spawnia deleted the allow-pushing-to-node-list branch February 16, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants