Typescript: Cannot annotate a field in constructor as null using jsdoc @type

Created on 7 Oct 2020  ·  7Comments  ·  Source: microsoft/TypeScript


TypeScript Version: 4.0.2


Search Terms: jsdoc null type implicit any

Code

class Foo {
    constructor () {
        /** @type {number} */
        this.bar = 42

        /** @type {null} */
        this.foo = null
    }
}

Expected behavior:

Expected to have a class with two fields, bar of type number & foo of type null

I don't know why it doesn't understand null; using string | null works fine.

Actual behavior: Member 'foo' implicitly has an 'any' type.

Playground Link: https://www.typescriptlang.org/play?useJavaScript=true#code/MYGwhgzhAEBiD29oG8BQ0PWPAdhALgE4Cuw+8h0AFAJQrqaMD0AVC9AAL4CeADgKYocxALYAjfoQC+0FkwaMM+ABYBLCADoxYSgF5oAFgBMqBYtbsufQcmEgQMuWcYr1GgGaJo+uyDNTUKSA

Related Issues:

Bug

All 7 comments

I expected this to be caused by --strictNullChecks=false, but surprisingly the behavior exists irrespective of that setting 👀

For the following examples getWidenedTypeForAssignmentDeclaration returns any

/** @type {null} */
this.foo = null
/** @type {undefined} */
this.foo = undefined
function getWidenedTypeForAssignmentDeclaration(symbol: Symbol, resolvedSymbol ? : Symbol) {
    ...
    const widened = getWidenedType(addOptionality(type, definedInMethod && !definedInConstructor));
    if (filterType(widened, t => !!(t.flags & ~TypeFlags.Nullable)) === neverType) {
        reportImplicitAny(symbol.valueDeclaration, anyType);
        return anyType; 
    }
    return widened;
}

this type resolution _conflicts_ with the serializer, because getTypeFromTypeNode(existing) returns null or undefined from the type defintion, however resolved type is equal to any, therefore property declaration type will be any.

function serializeTypeForDeclaration(...) {
    if (type !== errorType && enclosingDeclaration) {
        const declWithExistingAnnotation = getDeclarationWithTypeAnnotation(symbol, enclosingDeclaration);
        if (declWithExistingAnnotation && !isFunctionLikeDeclaration(declWithExistingAnnotation)) {
            // try to reuse the existing annotation
            const existing = getEffectiveTypeAnnotationNode(declWithExistingAnnotation) !;
            if (getTypeFromTypeNode(existing) === type && existingTypeNodeIsNotReferenceOrIsReferenceWithCompatibleTypeArgumentCount(existing, type)) {
                const result = serializeExistingTypeNode(context, existing, includePrivateSymbol, bundled);
                if (result) {
                    return result;
                }
            }
        }
    }
   ....
}

Not sure, maybe getWidenedTypeForAssignmentDeclaration should return resolved type?

/cc @sandersn @andrewbranch

I can't speak for the serializer code, since it's much newer than getWidenedTypeForAssignmentDeclaration. However, the latter treats a lone null the same whether it comes from a type annotation or an initialiser -- it's regarded as an insufficient signal of intent and the code reports the error that @a-tarasyuk reports.

The correct fix is to treat an explicit type annotation differently from a null initialiser. However, I can't think of a good reason for declaring a property of type null. @Raynos can you explain what you were using it for? Until we have a good, commonly used reason, I don't feel like improving these semantics.

Meanwhile, I'm not sure what to do about the serialiser -- it prioritises the type annotation for historical reasons. Ideally it should use the same rules as getWidenedTypeForAssignmentDeclaration, and one easy but expensive way to make that happen is to always request the actual type instead of short-circuiting on type annotation.

I can think of one reason to do this that’s maybe not too farfetched: if you’re writing a subclass and the base has a property of type T | null, but that property is always null and not relevant to your subclass, you might want to declare and assign it null so consumers know not to look for anything there.

@Raynos can you explain what you were using it for?

This workflow is for converting an existing JavaScript class into a JSDoc --checkJS // @ts-check class.

  • I have a large existing class I want to introduce type safety to.
  • The constructor is the first method in the class block
  • A field is null in the constructor but is then updated to be some concrete object in some asynchronous method further down in the class
  • I want to add a type annotation to every this field in the constructor so I can get explicit type checking
  • Added @type {null} is valid for the constructor to type check and remove the red errors.
  • There should now be an error in some other method further down the class where assigning a concrete object to null is a type error
  • I can find that error and then update the type annotation in the constructor to be @type {null | MyThing}

I don't need a @type {null} but its a useful temporary annotation to remove red errors linearly in my text editor / tsc CLI as I'm going through a untyped file.

I looked for existing usage. null as a standalone annotation happens a fair amount in Typescript (and once in flow -- react-native), but never in Javascript. I looked at just the TS equivalent -- null on property declarations.

  • mobx and hubot uses it to show that a class chooses not to implement an interface's property usefully. (@andrewbranch this is your usage)
  • Firebase uses it on some mocks.
  • three uses it in three places, but I can't tell why. Maybe if I read the source it would be apparent.
  • ckeditor uses it in two places for consistency with other classes, even though they're not formally related by inheritance.

5 uses across all the TS source I searched is not a lot, so I think this is not a common pattern in Typescript either.

Also, compiler-directed typing is not the focus of Javascript support right now, and I believe we still don't officially recommend strictNullChecks: true with checkJS. I would recommend switching to Typescript for the fully typed experience.

I'm going to move this onto the backlog in case somebody wants to try it. We can evaluate whether the fix is worth it then. The change will be contained within getWidenedTypeForAssignmentDeclaration -- but that is a mess of a function, so I predict the change will make the code even messier.

Could we reduce confusion by warnings that annotating somethings it as not supported ?

/** @type {null} */ <= error here. null in @type not supported
this.foo = null

Some form of warning that using null with @type is treated the same as any and is thus not recommended or not useful.

Was this page helpful?
0 / 5 - 0 ratings