Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(eslint-plugin): [no-array-delete] add new rule#8067

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

StyleShit
Copy link
Contributor

Closes#4432

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@StyleShit!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

@netlifyNetlify
Copy link

netlifybot commentedDec 14, 2023•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit0004c10
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/659ae4373463650008d12cfd
😎 Deploy Previewhttps://deploy-preview-8067--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		1 paths audited
Performance: 98 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify site configuration.

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

A lovely start, thanks for working on this! The foundation is very solid 💪 just some touchups & more testing mostly.

Spongebob ripping off his shirt to reveal an extreme amount of muscles

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelDec 15, 2023
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelDec 17, 2023
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Getting there! A blocking request on thefix/suggestion difference. And some more testing I thought of just now. Thanks!~

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelDec 29, 2023
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelDec 30, 2023
@StyleShit
Copy link
ContributorAuthor

not sure why the CI is failing. doesn't seem to be related to this PR 🤷

@JoshuaKGoldberg
Copy link
Member

Yeah the CI failures are fixed onmain, sorry about that 😞

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Progress!

Comment on lines 74 to 76
if (node.argument.type !== AST_NODE_TYPES.MemberExpression) {
return null;
}

Choose a reason for hiding this comment

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

[Cleanup] I can see why this code helps -node.argument loses its type refinement (narrowing) inside this closure. But it's a duplicate statement that shouldn't be necessary.

I think a cleaner approach would be to storenode.argument in a variable. Then we wouldn't have to worry about its type being misunderstood by TypeScript later on.

const{ argument}=node;if(argument.type!==AST_NODE_TYPES.MemberExpression){return;}
Suggested change
if(node.argument.type!==AST_NODE_TYPES.MemberExpression){
returnnull;
}

Aside: I haven't looked deeply intomicrosoft/TypeScript#56908 to see whether it would apply to a property of a parameter.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I actually tried it and it didn't work for some reason. now it works 😅

TS is afraid of you

JoshuaKGoldberg reacted with laugh emoji

Choose a reason for hiding this comment

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

Spongebob laughing evilly over dramatic purple-lit black underwater sky

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJan 4, 2024
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelJan 7, 2024
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Looks fantastic, thanks! 🔥

The Simpsons family gathering and playing tribal music in front of a burning effigy of some sort. Caption: '(playing upbeat music)'

@JoshuaKGoldbergJoshuaKGoldberg merged commite0f591e intotypescript-eslint:mainJan 9, 2024
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsJan 17, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rule proposal: no-array-delete
2 participants
@StyleShit@JoshuaKGoldberg
[8]ページ先頭
©2009-2025 Movatter.jp