Record: optimise filterWithIndex (conservative) and traverseWithKey (less object creations) (fixes #711)

[sledorze]

No description provided.

[sledorze]

@gcanti I still have the problem of running the tests locally (related to Jest
as depicted here #715
).
So blindly guessing and using the CI to fix issues, sorry for the noise..

[gcanti]

I still have the problem of running the tests locally

Ah! Adding

"jest": {
  "testURL": "http://localhost/"
}

in package.json does solve the problem?

[gcanti]

@sledorze actually we could upgrade to [email protected]

[gcanti]

What about

export function filterWithIndex<A>(fa: Record<string, A>, p: (key: string, a: A) => boolean): Record<string, A> {
  const r: Record<string, A> = {}
  let changed = false
  for (const key in fa) {
    if (fa.hasOwnProperty(key)) {
      const a = fa[key]
      if (p(key, a)) {
        r[key] = a
      } else {
        changed = true
      }
    }
  }
  return changed ? r : fa
}

[sledorze]

It's doing all work in case of noop (obj création plus assigns (n log n))

[gcanti]

noop is unlikely in a filter* function so in the most common case looks like you are doing even more work

[sledorze]

I m fine with the simpler approach.
Afk this morning (on phone) so expect a change later.

[gcanti]

This test doesn't look different from the previous one

[sledorze]

Was to trigger another case for coverage

[sledorze]

@gcanti yes adding

"jest": {
  "testURL": "http://localhost/"
}

Solves the problem (I have another one popping, however)

[gcanti]

FYI we are upgrading jest in the other PR #715 (comment)

[sledorze]

@gcanti not sure how I can fix that coverage issue

[gcanti]

@sledorze did you try open coverage/lcov-report/index.html?

EDIT: specifically coverage/lcov-report/Record.ts.html

[gcanti]

ah sorry, do you mean this line right?

[sledorze]

yes

[gcanti]

this seems to work

const x = Object.assign(Object.create({ c: true }), { a: 1, b: 'foo' })
assert.deepStrictEqual(R.filter(x, isNumber), { a: 1 })

[sledorze]

thanks @gcanti