Skip to content

eslint rule: no-deep-destructuring should handle ArrayPattern#35698

Merged
samouri merged 3 commits intoampproject:mainfrom
samouri:no-deep-arr
Aug 17, 2021
Merged

eslint rule: no-deep-destructuring should handle ArrayPattern#35698
samouri merged 3 commits intoampproject:mainfrom
samouri:no-deep-arr

Conversation

@samouri
Copy link
Copy Markdown
Member

@samouri samouri commented Aug 16, 2021

summary
Partial for #35694

Explicitly bans these kinds of patterns:

const [[x], y] = arr;
const [{x}] = arr

@amp-owners-bot
Copy link
Copy Markdown

Hey @erwinmombay, @jridgewell! These files were changed:

build-system/eslint-rules/no-deep-destructuring.js

@@ -170,6 +170,7 @@ function _createBody(head, base, prs) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rule can be disabled in build-system/.eslintrc.js

Copy link
Copy Markdown
Member Author

@samouri samouri Aug 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we do that? I guess depends on our reason for banning:

  • Difficult to read nested destructures: applies to both build-system and src
  • Difficult to create transforms on Pattern nodes (only applies to src)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they're all difficult to read, but the only reason I care is the difficulty to create transforms.

return countGenerators[type]();
}, [type]);

// eslint-disable-next-line local/no-deep-destructuring
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@samouri samouri merged commit 00febc9 into ampproject:main Aug 17, 2021
@samouri samouri deleted the no-deep-arr branch August 17, 2021 02:13
caroqliu pushed a commit to caroqliu/amphtml that referenced this pull request Aug 17, 2021
…ject#35698)

* eslint rule: no-deep-destructuring should handle ArrayPattern

* Ignore pre-existing destructures

* fix instead of ignore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants