Skip to content

feat: add iterator function to NodeList and NamedNodeMap#634

Merged
karfau merged 7 commits into
xmldom:masterfrom
Ponynjaa:feature/633
Mar 17, 2024
Merged

feat: add iterator function to NodeList and NamedNodeMap#634
karfau merged 7 commits into
xmldom:masterfrom
Ponynjaa:feature/633

Conversation

@Ponynjaa

Copy link
Copy Markdown
Collaborator

Closes #633

Comment thread package-lock.json
@codecov

codecov Bot commented Mar 17, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.96%. Comparing base (91039af) to head (4adbbf3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #634      +/-   ##
==========================================
+ Coverage   93.89%   93.96%   +0.06%     
==========================================
  Files           8        8              
  Lines        2048     2070      +22     
  Branches      532      536       +4     
==========================================
+ Hits         1923     1945      +22     
  Misses        125      125              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@karfau

karfau commented Mar 17, 2024

Copy link
Copy Markdown
Member

Can you please also add a test for these?
It doesn't match everywhere, but I'm trying to reflect the prototype function names in the lib/dom.js module by the files in the test/dom folder.

@karfau

karfau commented Mar 17, 2024

Copy link
Copy Markdown
Member

There currently is a restriction to only use ES5 inside the lib folder.
Is there a way to implement this with ES5?
(To get rid/loosen that restriction has a lot of aspects, and I'm currently not able to really decide anything regarding this.
Thank you for being the first to reply to the related poll I pinned in the discussion.)

@Ponynjaa

Copy link
Copy Markdown
Collaborator Author

I added tests and rewrote the iterator to be ES5 compatible. As you can see it's way more code for the same thing but I guess it works...

Comment thread test/dom/node-list.test.js Fixed
Comment thread test/dom/node-list.test.js Fixed
Comment thread test/dom/node-list.test.js Fixed
Comment thread test/dom/node-list.test.js Fixed
Comment thread test/dom/named-node-map.test.js Fixed
Comment thread test/dom/named-node-map.test.js Fixed
Comment thread test/dom/named-node-map.test.js Fixed
Comment thread test/dom/named-node-map.test.js Fixed
@karfau karfau changed the title 633: added iterator function to NodeList and NamedNodeMap feat: add iterator function to NodeList and NamedNodeMap Mar 17, 2024

@karfau karfau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think when prefixing the unused item variables, that eslint complains about in the tests with _ eslint should stop complaining.
Great job!

@Ponynjaa

Copy link
Copy Markdown
Collaborator Author

@karfau done 😉

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.

[feature-request] Make Element::childNodes and Element::attributes iterable

3 participants