Repository: TypeScript
Issue: Design Meeting Notes, 7/16/2024
Labels: Design Notes

# No Suspect Truthy Checks

https://github.com/microsoft/TypeScript/pull/59217

* Team told us they wrote `if (/some regex/)` accidentally - but so many expressions are always truthy.
* We already do stuff like disallowing `x === { prop: obj }` because it's always false.
* This PR disallows stuff like `if ([some, array, contents])` and similar things.
* Unveiled 50 different breaks in the top 800!
    * Sounds very breaky! But **all** of these breaks are unambiguously good.
* Stuff like
    * `foo > bar ?? baz` where the user assumed it was equivalent to `foo > (bar ?? baz)`.
* Feels linty, but it's so good (and clearly projects using linters haven't caught these yet).
    * `no-constant-binary-expression` is the closest ESLint rule we're aware of, but it doesn't do all of this.
* What should the error message be?
    * This appears to be unintentional because the expression on the left is always/never nullish.
    * This expression is always nullish and that expression will never/always be observed.
        * Right operand is unreachable because the left side is always/never nullish.
* Aside: "what's the deal with 'left-hand side'!? The hands aren't disambiguating anything!"
* Want to highlight the entire left side because it shows the precedence.
    * Could special-case some expressions.
        * "This binary expression is never nullish. Are you missing parentheses?"
* Feels like "look at the left side" is usually the right thing to do.
* We like "this kind of expression".

# 

#57103

* In 5.5, we started checking to ensure that signatures *without* `this` type predicates are not assignable to signatures with `this` type predicates..
* We thought the fact that we didn't do this check was an oversight, but...maybe not?


  ```ts
  export interface Foo {
    isBlah(): this is { blah: true };
  }

  let obj: Foo = {
    isBlah(): this is { blah: true } {
    //        ~~~~
    // Error! A 'this' type is available only in a non-static member of a class or interface.
      return true;
    }
  }
  ```
* You can't refer to the `this` type in an object literal.
* So...it's impossible to implement an object with this signature.
* But wait... in that position, `this` is not a *type*, it's a *value*.
    * And things work correctly on use-site.
    * So this is probably just a bug on where we try to check if `this` is used correctly.
* Also what happens if you write...

  ```ts
  let isBlah = obj.isBlah;
  if (isBlah()) {
    // ...
    // Should `window.blah` exist? 😄
  }
  ```
    * But really, nothing happens.
    * (Someone should write a test though!)

# Flag for Side-Effect Imports

https://github.com/microsoft/TypeScript/pull/58941

* If you write `import 'foo'`, and `foo` doesn't exist, TypeScript doesn't error.
* Typically people do this to bring `.css` files into an environment.
    * So for those people, it's a "feature".
* This flag (`resolveSideEffectImports`) tries to at least resolve the path and bring it into the program.
    * We don't read arbitrary files into the program though.
* Code change is pretty small.
    * Most of the test failures are CSS imports.
    * On DefinitelyTyped, it's people reading `.css` files.
* Why not just resolve but *not* bring it into the program? Because the way things work now, it *will* error on `.css` files unless you have `declare module "*.css"`.
    * We'd need to start creating file-watchers for non-existent paths?
* Why don't we always turn this on? Seems crazy that we don't error when you write a path.
    * Mainly backwards-compatibility.
* This all probably predates wildcard modules.
* So this always brings the JS/TS file into the program. What happens when you import a JS file without a `.d.ts` file?
    * (without `allowJs`)
    * Should we make that an `implicitAny` error?
        * Probably shouldn't be an error, right?
        * Well... yes, seems like we are strict here, but the file should just add an `export {}`.
* We like it in general.
* Should it be `resolve` or `check`?
    * So `checkSideEffectImports`?
    * `checkSideEffectImportSpecifiers` - `Specifiers` because we already check if it resolves, but we don't check that the specifier is valid.