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
Fixes incorrectly applied .forEach #2554
Conversation
`.forEach` was incorrectly made available to all DOM Iterators in core-js. This has now been fixed in the package: zloirock/core-js#988 The recommended approach for HTMLCollections objects is to use `Array.from` (https://developer.mozilla.org/en-US/docs/Web/API/HTMLCollection). This has been applied to the code to support the `.forEach` function. This was originally noticed when updating the yarn.lock file in PR #2551. Credits: @ollietreend for finding the solution.
@@ -36,7 +36,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {}; | |||
id: $select.id, | |||
source: function (query, syncResults) { | |||
var results = [] | |||
$select.options.forEach(function ($el) { | |||
Array.from($select.options).forEach(function ($el) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array.from is a method that's not in IE. Do we get this polyfilled by core JS?
Could also use the boring old for (var i ..
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh! 🙈 Good spot @kevindew.
This should do the trick then:
var $el
for (var i = 0; i < $select.options.length; i++) {
$el = $select.options[i]
console.log($el)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like core js polyfills it: https://github.com/zloirock/core-js/blob/master/packages/core-js/modules/es.array.from.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Great stuff 👍 Always fun when we actually get to use other enumerators than the basic for loop. |
.forEach
was incorrectly made available to all DOM Iterators in core-js. This has now been fixed in the package: zloirock/core-js#988The recommended approach for HTMLCollections objects is to use
Array.from
(https://developer.mozilla.org/en-US/docs/Web/API/HTMLCollection). This has been applied to the code to support the.forEach
function.This was originally noticed when updating the yarn.lock file in PR #2551.
Credits: @ollietreend for finding the solution.