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): split no-empty-object-type out from ban-types and no-empty-interfaces#8977

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

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg commentedApr 23, 2024•
edited
Loading

BREAKING CHANGE:
Changes the default options of a lint rule

PR Checklist

Overview

Per#8700 (comment), splits out the banning of{} from theban-types rule into a newno-empty-object-type rule.

Screenshot of the rule's new report message on a {} type in a VS Code hover card. Quick fixes are available.

danielroe reacted with thumbs up emojidanielroe reacted with heart emoji
@JoshuaKGoldbergJoshuaKGoldberg added this to the8.0.0 milestoneApr 23, 2024
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@JoshuaKGoldberg!

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.

@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedApr 23, 2024•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit7dc88d7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 31 targets

Sent with 💌 fromNxCloud.

@JoshuaKGoldbergJoshuaKGoldberg marked this pull request as ready for reviewApril 23, 2024 19:37
@@ -82,32 +85,6 @@ const defaultTypes: Types = {
'If you are expecting the function to accept certain arguments, you should explicitly define the function shape.',
].join('\n'),
},

// object typing
Object: {
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

We didn't explicitly discussObject, but I think it makes sense in the spirit of this change to switch it to being treated as another banned uppercase alias. I've never seen people confuseObject for{}.

kirkwaiblinger reacted with thumbs up emoji
Co-authored-by: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com>
Co-authored-by: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com>
@bradzacher
Copy link
Member

Mildly tangential - but I can envision the following options:

  • allowInTypeAliasWithName - for react codebases to allowtype MyProps = {}
  • allowInIntersection - to allowtype T = A & {} - wherein the{} falls away.
    • does this even need an option...? Probably should be the default?
kirkwaiblinger and JoshuaKGoldberg reacted with thumbs up emoji

@kirkwaiblinger

This comment was marked as resolved.

@codecovCodecov
Copy link

codecovbot commentedApr 23, 2024•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.34%. Comparing base(04f1938) to head(39870f1).

❗ Current head39870f1 differs from pull request most recent head7dc88d7. Consider uploading reports for the commit7dc88d7 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@##               v8    #8977      +/-   ##==========================================- Coverage   87.43%   87.34%   -0.09%==========================================  Files         386      387       +1       Lines       13069    13025      -44       Branches     3776     3769       -7     ==========================================- Hits        11427    11377      -50- Misses       1353     1356       +3- Partials      289      292       +3
FlagCoverage Δ
unittest87.34% <100.00%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown.Click here to find out more.

FilesCoverage Δ
packages/eslint-plugin/src/rules/ban-types.ts95.83% <ø> (-4.17%)⬇️
...es/eslint-plugin/src/rules/no-empty-object-type.ts100.00% <100.00%> (ø)

... and33 files with indirect coverage changes

@JoshuaKGoldberg
Copy link
MemberAuthor

JoshuaKGoldberg commentedApr 26, 2024•
edited
Loading

standalone rule for v7

I like that a lot! Regardless of what we choose forno-empty-interface, this lets us get it released faster. Excellent. I'll get that PR up now. Edit: actually, no, I'll wait a bit - maybe it wouldn't make sense.

@JoshuaKGoldbergJoshuaKGoldberg marked this pull request as draftApril 26, 2024 13:42
Copy link
Member

@bradzacherbradzacher left a comment

Choose a reason for hiding this comment

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

general logic LGTM

@JoshuaKGoldbergJoshuaKGoldberg marked this pull request as ready for reviewMay 8, 2024 15:36
@JoshuaKGoldbergJoshuaKGoldberg changed the titlefeat(eslint-plugin): split no-empty-object-type rule out from ban-types rulefeat(eslint-plugin): split no-empty-object-type out from ban-types and no-empty-interfacesMay 12, 2024
@JoshuaKGoldbergJoshuaKGoldberg merged commitfd097ef intotypescript-eslint:v8May 12, 2024
58 checks passed
@JoshuaKGoldbergJoshuaKGoldberg deleted the no-empty-object-type branchMay 12, 2024 05:42
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsMay 21, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@mattpocockmattpocockmattpocock left review comments

@kirkwaiblingerkirkwaiblingerkirkwaiblinger approved these changes

@bradzacherbradzacherbradzacher approved these changes

@Josh-CenaJosh-CenaAwaiting requested review from Josh-Cena

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

Successfully merging this pull request may close these issues.

5 participants
@JoshuaKGoldberg@bradzacher@kirkwaiblinger@mattpocock@Josh-Cena
[8]ページ先頭
©2009-2025 Movatter.jp