Skip to content
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

Rule proposal: Named object destructuring #2401

Open
Richienb opened this issue Jul 16, 2024 · 7 comments
Open

Rule proposal: Named object destructuring #2401

Richienb opened this issue Jul 16, 2024 · 7 comments

Comments

@Richienb
Copy link
Contributor

Description

Readability/consistency

It took me a moment to figure out what this line does:
https://github.com/sindresorhus/open/blob/2ea66da8e8b20880d235447cf4c94ba275da6a5a/index.js#L307

Fail

const {[a]: b} = c;

I think we should still detect it here since I believe consistency should be sacrificed for readability (does someone remember what that 3-step or so style guide was called?).

const {d} = g;
const {[a]: b} = c;
const {f} = h;

Pass

const b = c[a];
const {d} = g;
const b = c[a];
const {f} = h;

Proposed rule name

no-unreadable-object-destructuring

Additional Info

No response

@fregante
Copy link
Collaborator

fregante commented Jul 25, 2024

I agree, IMHO only very basic destructuring should ever be allowed, anything else is way harder to parse than the basic version.

Unicorn already has a related rule but just for arrays:

https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-unreadable-array-destructuring.md

@sindresorhus
Copy link
Owner

👍 I think it would make sense to add a no-unreadable-object-destructuring rule.

@sindresorhus
Copy link
Owner

Would be good to try to think of other anti-patterns that this rule could also catch.

@fisker
Copy link
Collaborator

fisker commented Aug 1, 2024

The computed destructuring is readable to me, but people may get confused by some of the following cases

  1. Renamed

    const  {a: renamed} = b
  2. Default values

    const {a = b} =c
  3. Deep destructuring

    const {
    	a: {
    		b
    	}
    } = d
  4. Array destructuring inside object destructuring

    const {
    	a: [b]
    } = d

Combined all cases above

const {
	[a]: {
		[b]: c = d,
		e: [f]
	} = {}
} = g

@fregante
Copy link
Collaborator

fregante commented Aug 1, 2024

3 and 4 are visually same, one is {b} and the other one is [b], neither one is readable 👍

Renames and default values are not super common, but they're very useful when setting defaults from options objects:

const {
  enabled = true,
  id,
  depth = 1,
  replacements = []
} = options;

Maybe combining both still isn't ideal:

const {
  enabled: isEnabled = true,
  id: name,
  depth = 1,
  replacements: array = []
} = options;

Combining it with inline types is particularly lethal, but that's another issue.


I suppose this rule should also apply to function parameters:

function foo({
	[a]: {
		[b]: c = d,
		e: [f]
	} = {}
}) {}

@sindresorhus
Copy link
Owner

Renames and default values are very useful and should be allowed.

Deep restructuring should be allowed to a certain level. Maybe two levels down.

Array destructuring inside an object is pretty unreadable and should not be allowed.

@fisker
Copy link
Collaborator

fisker commented Aug 1, 2024

Deep destructuring and default value can also confusing when the right side has multiple properties, even it's "simple".

Shorthand
const {
	a = {
		b,
		c
	}
} = d
const {
	a: {
		b,
		c
	}
} = d
const {
	a: e = {
		b,
		c
	}
} = d
Longhand
const {
	a = {
		b: e,
		c
	}
} = d
const {
	a: {
		b: e,
		c
	}
} = d
const {
	a: {
		b = e,
		c
	}
} = d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants