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.

Sign up for GitHub

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

Jump to bottom

chore(lint): re-enable @typescript-eslint/ban-types #3464

Merged
merged 10 commits into from
Dec 23, 2022
Merged

chore(lint): re-enable @typescript-eslint/ban-types #3464

merged 10 commits into from
Dec 23, 2022

Conversation

KATT
Copy link
Member

@KATT KATT commented Dec 23, 2022 •
edited

Context / Background: https://twitter.com/JoshuaKGoldberg/status/1606070961182228481

🎯 Changes

  • Re-add @typescript-eslint/ban-types
  • Add eslint-ignore on any interop files
  • Fix the rest

✅ Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@vercel
Copy link

vercel bot commented Dec 23, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
next-prisma-starter ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 23, 2022 at 1:24AM (UTC)
og-image ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 23, 2022 at 1:24AM (UTC)
todomvc 🔄 Building (Inspect) Dec 23, 2022 at 1:24AM (UTC)
www ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 23, 2022 at 1:24AM (UTC)

Base automatically changed from issues/3453-better-yet to main December 23, 2022 00:14
@KATT KATT marked this pull request as ready for review December 23, 2022 00:15
@KATT KATT changed the title chore: try re-adding @typescript-eslint/ban-types chore: add @typescript-eslint/ban-types Dec 23, 2022
@KATT KATT changed the title chore: add @typescript-eslint/ban-types chore(lint): add @typescript-eslint/ban-types Dec 23, 2022
@KATT KATT changed the title chore(lint): add @typescript-eslint/ban-types chore(lint): skip disabling @typescript-eslint/ban-types Dec 23, 2022
KATT
KATT commented Dec 23, 2022
View reviewed changes
package.json Outdated Show resolved Hide resolved
@KATT KATT changed the title chore(lint): skip disabling @typescript-eslint/ban-types chore(lint): re-enable @typescript-eslint/ban-types Dec 23, 2022
sachinraja
sachinraja approved these changes Dec 23, 2022
View reviewed changes
Copy link
Member

@sachinraja sachinraja left a comment

Choose a reason for hiding this comment

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

looks good! had some concerns about if this would affect what TypeScript shows in the tooltip when you hover over a variable (the string representation of a type) but that seems to still be fine

@KATT KATT enabled auto-merge (squash) December 23, 2022 08:27
@KATT KATT merged commit 1e32baf into main Dec 23, 2022
@KATT KATT deleted the issues/3453-better-yet-yet branch December 23, 2022 08:30
nfabredev pushed a commit to nfabredev/trpc that referenced this pull request Dec 23, 2022
@KATT @nfabredev
chore(lint): re-enable @typescript-eslint/ban-types (trpc#3464)
a02430d
JoshuaKGoldberg
JoshuaKGoldberg reviewed Dec 23, 2022
View reviewed changes
Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Lovely, thank you for taking action on this! It's looking great. ☺️

Just left a few comments inline where files that you might be able to undo the eslint-disables in.

packages/tests/server/interop/index.test.tsx
@@ -1,3 +1,5 @@
/* eslint-disable @typescript-eslint/ban-types */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason for this one? I tried switching the {}s to objects in it and didn't see any build failures.

packages/tests/server/interop/react/__testHelpers.tsx
@@ -1,3 +1,5 @@
/* eslint-disable @typescript-eslint/ban-types */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason for this one? I tried switching the {} to object in it and didn't see any build failures.

packages/tests/server/interop/regression/issue-949-inferProcedureInput.test.ts
@@ -1,3 +1,5 @@
/* eslint-disable @typescript-eslint/ban-types */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason for this one? I tried switching the {}s to objects in it and didn't see any build failures.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg left review comments

@sachinraja sachinraja sachinraja approved these changes

@trpc-bot trpc-bot Awaiting requested review from trpc-bot

Assignees
No one assigned
Labels
♻️ autoupdate @trpc/react-query @trpc/server
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@KATT @JoshuaKGoldberg @sachinraja