Not sure if this is appropriate to post this here and if not, I apologize.
I just created a PR to add map
functions to the Node and Fragment classes and would love to get some feedback.
Thank you!
Not sure if this is appropriate to post this here and if not, I apologize.
I just created a PR to add map
functions to the Node and Fragment classes and would love to get some feedback.
Thank you!
It’s not necessary to announce PRs here—I’ll get an email notification from GitHub.
In the PR you mentioned:
I think that this kind of simple utility is best implemented locally in the codebase that needs it.
Do you have an example of what you mean? I ask because in our codebase I’m seeing a lot of:
let childNodes = [];
node.forEach(childNode => childNodes.push(childNode));
and doing similar things for fragments.
Would it be possible to expose the child nodes in the Node
and Fragment
classes? They are already exposed, as shown above, this would just provide a simpler way to access them.
Write mapNode(node, f)
as a local function in your codebase, then use it wherever you’d want to use Node.map
.
Yes. For a long time I figured I may have to support a tree representation to make nodes with tons of children fast enough, so the interface was designed to not lock us into an array representation. But it turns out the browser DOM performance falls apart way before the array searching and rebuilding causes any real performance issues, so I guess we’re at a point where I can decide that these are always going to be arrays. This patch makes them public.