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

Bug: [ban-types] Should suggest object, not Record<string, never> #5947

Closed
4 tasks done
JoshuaKGoldberg opened this issue Nov 8, 2022 · 1 comment · Fixed by #6079 or #6209
Closed
4 tasks done

Bug: [ban-types] Should suggest object, not Record<string, never> #5947

JoshuaKGoldberg opened this issue Nov 8, 2022 · 1 comment · Fixed by #6079 or #6209
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Nov 8, 2022

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=4.8.4&jsx=true&sourceType=module&code=CYUwxgNghgTiAEEQBd4CsBc8CuA7A1rgPYDuuA3AFBKoC28AvPAN7wB0HAFGvFAM4sAvgEp4gqkA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6AIwEMnajTUMkRNGgB7aJHBgAviGlA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA

Repro Code

declare let j: unknown;
let m = { ...(j as {}) };

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/ban-types": "error",
  },
};

tsconfig

No response

Expected Result

From @RyanCavanaugh in #5018:

Now that object has been shipping in TS for several years, this seems like a safe suggestion in lieu of Record's messy side effects

From @bradzacher in #5018 (comment):

The issue with object as a suggested replacement has always been that it's really hard use (microsoft/TypeScript#21732).

#21732 -> github.com/microsoft/TypeScript/pull/50666 has been shipped, so the major flaws mentioned should be gone now.

Proposal: let's suggest object:

- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
+ If you want a type meaning "any object", you probably want `object` instead.

I'm unsure what to do about {}. On the one hand:

Having typescript-eslint, out of the box, ban you from writing a type that TypeScript itself is inferring under totally normal operation, is awkward.

On the other hand, people do mistake it for "any object". 🤔

Actual Result

Right now we still recommend:

Don't use `{}` as a type. `{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead. 

Additional Info

Originally suggested by @RyanCavanaugh in #5018.

Versions

package version
@typescript-eslint/eslint-plugin 5.42.1
@typescript-eslint/parser 5.42.1
TypeScript 4.8.4
ESLint 8.15.0
node web
@JoshuaKGoldberg JoshuaKGoldberg added bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Nov 8, 2022
@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: [ban-types] Should no longer ban {} or object Bug: [ban-types] Should no longer ban object Nov 8, 2022
@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: [ban-types] Should no longer ban object Bug: [ban-types] Should suggest object Nov 8, 2022
@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: [ban-types] Should suggest object Bug: [ban-types] Should suggest object, not Record<string, never> Nov 8, 2022
@bradzacher bradzacher added enhancement New feature or request accepting prs Go ahead, send a pull request that resolves this issue and removed bug Something isn't working triage Waiting for maintainers to take a look labels Nov 8, 2022
@bradzacher
Copy link
Member

yup lets do this now that the new TS version has fixed the in problems!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement New feature or request good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants