Design Meeting Notes, 7/16/2024

[DanielRosenwasser]

No Suspect Truthy Checks

#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?

  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...

  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

#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.

[ehoogeveen-medweb]

checkSideEffectImportSpecifiers - Specifiers because we already check if it resolves, but we don't check that the specifier is valid.

I'm struggling to parse this - what's the difference between checking if it resolves and checking that it's valid? If TypeScript already checks whether a given specifier resolves, I would expect that to mean it errors if the corresponding file doesn't exist.

[jakebailey]

what's the difference between checking if it resolves and checking that it's valid? If TypeScript already checks whether a given specifier resolves, I would expect that to mean it errors if the corresponding file doesn't exist.

Checking to see if it resolve is what the new flag would do. Today, you can write:

import "hello-world-this-is-fake";

And it won't error at all. If it does exist, we technically will resolve it, sort of, to pull its referenced files in, but we won't resolve it enough to actually issue an error or anything. So it's down to "semantics" on what the name should be.