commenthol / date-holidays-parser

parser for worldwide holidays
ISC License
46 stars 23 forks source link

Fix: Use `for of` instead of `for in` #27

Closed leonidasv closed 3 years ago

leonidasv commented 3 years ago

for in is prone to collisions with Array prototype extensions that are fairly common in Typescript settings.

This should solve issues like: https://github.com/commenthol/date-holidays/issues/168 https://github.com/commenthol/date-holidays-parser/issues/18

It does the same as PR https://github.com/commenthol/date-holidays-parser/pull/19 but for all other loops.

commenthol commented 3 years ago

Hi, I'd like to take the oportunity here to clarify the difference between for ... in and for ... of on arrays. Hope not bothering you about that.

for ... in loops over the array using the "keys" of the array

const a = ['a', 'b', 'c'] 
for (let i in a) {
  console.log(i)
}
// 0, 1, 2

as long as you are not doing monkey-patching on Array (with is not recommended) all will work as expected.

for ... of loops differently using the "content" of the array

const a = ['a', 'b', 'c'] 
for (let i of a) {
  console.log(i)
}
// 'a', 'b', 'c'

Your changes will always returns undefined as

const a = ['a', 'b', 'c'] 
for (let i of a) {
  console.log(a[i])
}
// undefined, undefined, undefined

My recommendation is don't monkey-patch global objects as this will cause side-effects, try extending from those objects instead:

// extending with own class is the better option ...
class MyArray extends Array {
  get doubleLength () {
    return this.length * 2
  }
}

const better = new MyArray('a','B','c')
print('extends', better)

// ... than Monkey-patching which causes side-effects
Object.defineProperty(Array.prototype, 'doubleLength', {
  configurable: true,
  enumerable: true,
  get: function () {
    console.log(this)
    return this.length * 2
  }
})

const monkey = new Array('a','B','c')
print('monkey', monkey)

function print (name, a) {
  console.log('------', name)
  console.log(a.doubleLength, a)

  for (let i in a) {
    console.log(i)
  }

  for (let i of a) {
    console.log(i)
  }
}

I did the changes for you in the esm branch, but I really would recommend to look where you monkey-patch Array with some "custom" methods.