Property assignments in Typescript

[sandersn]

Allow JS-container (expando) property assignments in Typescript, but only for function declarations and function/arrow expressions used as initialisers of const variables.

For example, now you can write the same code as you would in Javascript to make a callable object that also has properties:

function borscht(ingredients: any[]) {
   // let's just skip straight to the ???
}
borscht.and = 'burgers'

Previously you need to merge a namespace:

function borscht(ingredients: any[]) {
   // let's just skip straight to the ???
}
namespace borscht {
  export var and = 'burgers'
}

Note that this only works in the value space. To specify a callable object with properties, you should still use object literal notation (or merge a namespace):

declare const borscht = {
  (ingredients: any[]): Food
  and: string
}

Fixes #15868

[sandersn]

getInitializerTypeFromSpecialDeclarations is only called once, so I just made the caller check for undefined. You should turn on Ignore Whitespace to read this review.

[sandersn]

I didn't want to create another wrapper/worker pair, but I couldn't come up with a better way to special-case the isJS check just for property assignments. Any ideas?

[ghost]

Do we have an issue open for the following?

function f() {}
f.p = { x: "" };
const { p } = f;
if (1 + 1 < 2) {
    f.p = { x: 0 };
}
f.p.x = 0;
p.x.toUpperCase(); // No compile error, crash at runtime

A hole like that might be acceptable for JS expando objects, but I don't know about having this in .ts files.

[ghost]

Completions will have to be updated to allow f.x as a new identifier location when f is a function. That could be a separate issue.

[sandersn]

I opened #26372 to track the type hole you found.

[ghost]

Nit: In both places isTsFunctionSymbol is called, you just checked isInJavaScriptFile to be false. So maybe it could just be isFunctionSymbol.

[ghost]

Tests seem to pass without this line -- what's this protecting against?

[sandersn]

FindAllReferences relies on this function, and perhaps other services do. Without this line, a few fourslash tests fail, pointing out that getSpecialPropertyAssignmentKind now recognizes any property assignment in TS files, even those that aren’t actually expando assignments.

The left side of the or restores the old behavior for JS, and the right side adds to it for TS expando assignments. However, the right side isn’t tested. I need to add a case similar to the existing JS ones.

[RyanCavanaugh]

Can you briefly convince me that symbol.exports is always defined?

[sandersn]

Inside the containing if, symbol is the commonjs equivalent of export =: module.exports = { ... }. So it's guaranteed to have exports -- they are the contents of the module.exports = initializer, and we restricted the initializer's type with type.flags & TypeFlags.Object.

[kaleb]

This does not work when the property is a well-known symbol.

function count(start = 0) {
  while (true) {
    yield start;
    start += 1;
  }
}
count[Symbol.iterator] = () => count();

[sandersn]

@kaleb that’s not part of the current design; it only applies to property accesses, not element accesses. And it doesn’t handle computed property names at all, even well-known symbols. Can you open a separate issue to track that?

[bcherny]

Should TSC verify that every branch leads to an assignment in this case?

type WarnUser = {
  (warning: string): void
  wasCalled: boolean
}

const warnUser: WarnUser = (warning: string) => {
  if (warnUser.wasCalled) {
    return
  }
  warnUser.wasCalled = true
  alert(warning)
}
if (Math.random() < .5) {
  warnUser.wasCalled = false
}
// No error

[sandersn]

@bcherny Your example actually doesn't use control flow much, which means that we won't be able to issue a use-before-def error. Let's look at each use of wasCalled:

1. if (warnUser.wasCalled) is inside a closure. It's going to use the declared type, which comes from type WarnUser. Notably, WarnUser.wasCalled doesn't contain undefined and isn't optional.
2. warnUser.wasCalled is also inside a closure. It doesn't count as a declaration since it's not in the same scope as warnUser. It will also use the declared type.
3. Initialisation inside an if. This counts as a declaration so of course won't warn.
   3a. Before the if. There aren't any assignments or ifs here but the declared type isn't optional and doesn't contain undefined, so no error.
   3b. After the if. There is an if with an assignment inside, but they don't change the declared type.

You can do two [interesting] things to change this:

1. Remove the type annotation. The type of warnUser remains the same, but is now anonymous. The declared type is now based on checking warnUser's initialiser, which causes control flow to check warnUser.wasCalled = false, which it treats as an assignment. Now, as desired, (3a) and (3b) give a use-before-def error and you can get (3b) to be correct by adding an else-branch+assignment to the if.
2. Make WarnUser.wasCalled either optional or contain undefined in the type. You'd think this would make it behave the same as with the type annotation, but it has the same control flow behaviour as the original example — you get the declared type almost all the time. The type contains undefined, though, so the checker forces you to narrow that out before using the boolean. It just doesn't give a use-before-def error.

[bcherny]

@sandersn Thanks for the super detailed explanation of how the typechecker thinks about this example!