sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
3.98k stars 361 forks source link

Rule proposal: `consistent-conditional-object-spread` #2363

Open zanminkian opened 1 month ago

zanminkian commented 1 month ago

Description

When spreading an object conditionally, we have two style: ? : and &&. I prefer user consistently to use &&.

Pass

const name = Math.random() > 0.5 'foo' : undefined;
const student = {age: 12, ...(name&&{name})};

Fail

const name = Math.random() > 0.5 'foo' : undefined;
const student = {age: 12, ...(name ? {name} : {})};

Proposed rule name

consistent-conditional-object-spread

Additional Info

No response

fisker commented 1 month ago

This kind of like short circuit

foo && bar();

But I prefer

if (foo) {
    bar();
}

So I prefer the ternary version, but I use {...(name ? {name} : undefined)}

sindresorhus commented 1 month ago

I prefer && too in this case. There is really no benefit of the verbose ternary version. However, it should include an option to choose the style.

fisker commented 1 month ago

ONE benefit in werid JavaScript world.

({...(document.all && {foo: 1})})

// VS

({...(document.all ? {foo: 1} : undefined)})

Just kidding.

zanminkian commented 1 month ago

From my point of view, this rule only work when the variable name is the same as the property name. For example:

Work:

const student = {...(name?{name}:{})}
// or
const student = {...(name&&{name})}

Not work:

const student = {...(name?{age}:{})}
// or
const student = {...(name&&{age})}
fisker commented 1 month ago

this rule only work when the variable name is the same as the property name.

I don't think so, the name doesn't matter. Spread any falsely value in object literal does not add any property (of cause except HTMLDDA).

sindresorhus commented 1 week ago

Accepted