microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.89k stars 12.47k forks source link

TS2352: Suggest narrowing via type guard #51275

Open mrienstra opened 2 years ago

mrienstra commented 2 years ago

Suggestion

🔍 Search Terms

ts(2352), TS2352, neither type sufficiently overlaps, as unknown, instanceof, typeof, narrowing, type guard

Google search: site:github.com/microsoft/TypeScript/issues ("ts(2352)" OR "TS2352" OR "neither type sufficiently overlaps" OR "as unknown") ("instanceof" OR "typeof" OR "narrowing" OR "type guard")

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

Diagnostic message ts(2352) (Conversion of type '{0}' to type '{1}' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.) should suggest narrowing via type guard (instanceof, typeof). In scenarios where this is viable, it's much safer than x as unknown as SomeType. Ideally, this suggestion would only appear when applicable, e.g. when using as to narrow a type. In which case TS2352 would remain as-is, and a new diagnostic message would be created, to be shown when applicable.

📃 Motivating Example

Not sure this would be a "blog post" worthy change...

💻 Use Cases

While converting (from JS to TS) some code that I found on Stack Overflow, I saw TS2352 several times, in cases where I was attempting to use as to narrow DOM types, e.g:

In these cases, using instanceof type guards is a much better solution (at least to the best of my knowledge), e.g.

Presumably, it should be possible to look at a line like:

const x: CSSRule = foo.bar;
const y = x as CSSMediaRule;

... and then follow the chain of extends from CSSMediaRule to CSSRule:

interface CSSMediaRule extends CSSConditionRule {}
interface CSSConditionRule extends CSSGroupingRule {}
interface CSSGroupingRule extends CSSRule {}

(as per v4.8.4/lib/lib.dom.d.ts#L2573-L2662)

... but again, this may not be feasible to implement.

soullivaneuh commented 9 months ago

I currently do not have any issue using instanceof, here is my sample:

const switchTheme = () => {
  for (let s = 0; s < document.styleSheets.length; s++) {
    const styleSheet = document.styleSheets.item(s);
    const cssRules = styleSheet?.cssRules || new CSSRuleList();

    for (let r = 0; r < cssRules.length; r++) {
      const cssRule = cssRules.item(r);
      if (!(cssRule instanceof CSSMediaRule)) {
        continue;
      }

      const ruleMediaText = cssRule.media.mediaText;
      if (!ruleMediaText.match(/(prefers-color-scheme: \w+)/)) {
        continue;
      }

      const newRuleMediaText = ruleMediaText.includes('dark')
        ? ruleMediaText.replace('dark', 'light')
        : ruleMediaText.replace('light', 'dark')
      ;

      cssRule.media.deleteMedium(ruleMediaText);
      cssRule.media.appendMedium(newRuleMediaText);
    }
  }
}

Typescript version: 5.3.3

Typescript configuration:

{
  "compilerOptions": {
    "target": "ES2020",
    "useDefineForClassFields": true,
    "lib": ["ES2020", "DOM", "DOM.Iterable"],
    "module": "ESNext",
    "skipLibCheck": true,

    /* Bundler mode */
    "moduleResolution": "bundler",
    "allowImportingTsExtensions": true,
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx",

    /* Linting */
    "strict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noFallthroughCasesInSwitch": true
  },
  "include": ["src"],
  "references": [{ "path": "./tsconfig.node.json" }]
}