Check more cases in `prefer-object-from-entries`

[fisker]

We are going to add prefer-object-from-entries rule in #1308. As proposed in the original proposal, the following case should be reported too

const foo = {};
for (const [key, value] of pairs) {
	foo[key] = value;
}

But it's complicated, and I can't find a good solution for it. Opening this issue to keep track of it.

Note: As we agreed, we should only report cases that array is already key-value pairs, so it won't need a .map transform, see https://github.com/sindresorhus/eslint-plugin-unicorn/pull/1308#issuecomment-845751613

[jesse-savary]

Hey, not sure if this the right place to comment but I'm having a tough time understanding the purpose of this rule in context of my company's codebase. Take the following for example:

// modules would normally be generated at runtime
const modules = ['@expo/vector-icons', 'react-native-safe-area-context', 'react-native-reanimated']
const extraNodeModules = modules.reduce((acc, name) => {
  acc[name] = path.join(__dirname, 'node_modules', name)
  return acc
}, {})

I can understand how this looks a bit clunky but I don't follow how this is any cleaner:

const extraNodeModules = Object.fromEntries(
  Object.entries(modules).map(([key, value]) => [
    value,
    path.join(__dirname, 'node_modules', value),
  ]),
)

I figured it was something to do with performance but a quick benchmark showed Array.reduce as being almost twice as fast. Are we expecting Object.fromEntries to become faster over time or am I missing something?

Edit: was able to shorten the snippet to

const extraNodeModules = Object.fromEntries(modules.map((value) => [
  value, 
  path.join(__dirname, 'node_modules', value)
]))

This is shorter/better but still slower than reduce. Stylistically, is this closer to what the rule wants written?

[fregante]

This is shorter/better but still slower than reduce. Stylistically, is this closer to what the rule wants written?

Yes, that's the expected solution. Alternatively just use a for-of loop.

I'm having a tough time understanding the purpose of this rule

This rule is somewhat of an extension to https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-reduce.md, which one may or may not agree to.

It works better when there's no alteration/mapping to the arrays though, as can be seen in the examples:

https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-object-from-entries.md

[sindresorhus]

This is shorter/better but still slower than reduce.

.reduce is an older feature, so it's not surprising that it's more optimized. Object.fromEntries will get faster in time. I'm also suspecting what you consider "faster" is just a microbenchmark and does not reflect real-world performance.

[kitsunde]

Sorry to make a drive by comment, but I needed to test this because I have a real-world problem of dealing with arrays of large-ish objects with tens of thousands of rows in them as we have a data platform and have to worry about things like garbage collection and hot paths.

If anyone is interesting reviewing the performance impact of various ways of building objects over time, I included:

• iteratorMap
• Object.fromEntries (recommended)
• reduce
• for..of

https://jsbench.me/swlc79w08s/2

For various reasons with how bad JS is currently at doing iterative execution, I'm not very optimistic this will change for many years.