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

clean up Array.from in codebase (closes #3109) #3710

Closed

Conversation

kgeorgiou
Copy link

Removed all Array.from usages that were wrapping the results of a document.querySelectorAll.

The only Array.from usage left in the codebase is a legitimate one (converts an HTMLCollection to an array):
https://github.com/mozilla/foundation.mozilla.org/blob/2f2c49a8b8713cc9edcc2dd89ef5cd7abb5e34b3/source/js/main.js#L521-L523

I've also removed some unnecessary length checks before iterating the array returned by document.querySelectorAll. If no matches are returned by the query selector, an empty array is returned. [].forEach() wouldn't do anything so protecting the iteration on the length of the array being greater than 0 is excessive.

Closes #3109

Checklist

Tests

  • Is the code I'm adding covered by tests?

@Pomax Pomax self-requested a review October 1, 2019 00:58
@Pomax
Copy link
Contributor

Pomax commented Oct 1, 2019

Nice, thank you so much for doing this. If I can ask, how did you decided to work on this issue? (this repo is not on any "good for contributors" lists, and we're a small enough team to usually not ask for contributions because there's not enough spare time for people to walk contributors through the code to the point where they can meaningfully contribute).

@kgeorgiou
Copy link
Author

Hey @Pomax, no worries!

I love Mozilla & OSS so I really wanted to start my quest for Hacktorberfest here!

Let me know if there are any other issues I could help with.

Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

just one change.

@cadecairos cadecairos added this to the Mar 9 milestone Feb 26, 2020
@Pomax Pomax changed the base branch from master to array-from February 27, 2020 21:30
@Pomax
Copy link
Contributor

Pomax commented Feb 27, 2020

Apologies for the long delay, I've ported your changes over to #4262 and should be able to land that this sprint. Thanks again for helping.

@Pomax Pomax closed this Feb 27, 2020
@kgeorgiou
Copy link
Author

Apologies for the long delay, I've ported your changes over to #4262 and should be able to land that this sprint. Thanks again for helping.

No worries 🙌 Thanks!

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.

Clean up "Array.from" in codebase
3 participants