[Typescript] firebase-admin 4.1.1 doesn't compile on Typescript 2.1.5 with strictNullCheck on.

[ch...@hangar.com]

Hi,

In a project that is referencing 4.1.1 of firebase-admin, compiling on Typescript 2.1.5, we get a slew of errors around invalid index typings on the NotificationMessagePayload type.  Specifically, our tsconfig.json looks like this:

{

  "compilerOptions": {

    "module": "commonjs",

    "target": "es6",

    "removeComments": true,

    "strictNullChecks": true,

    "noUnusedLocals": false,

    "noUnusedParameters": false,

    "moduleResolution": "node",

    "sourceMap": true,

    "outDir": "build"

  },

  "compileOnSave": false,

  "include": [

    "src/**/*"

  ],

  "exclude": [

    "build",

    "node_modules"

  ]

}

and the errors in question look like this:

node_modules/firebase-admin/lib/index.d.ts(244,5): error TS2411: Property 'tag' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(245,5): error TS2411: Property 'body' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(246,5): error TS2411: Property 'icon' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(247,5): error TS2411: Property 'badge' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(248,5): error TS2411: Property 'color' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(249,5): error TS2411: Property 'sound' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(250,5): error TS2411: Property 'title' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(251,5): error TS2411: Property 'bodyLocKey' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(252,5): error TS2411: Property 'bodyLocArgs' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(253,5): error TS2411: Property 'clickAction' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(254,5): error TS2411: Property 'titleLocKey' of type 'string | undefined' is not assignable to string index type 'string'.

node_modules/firebase-admin/lib/index.d.ts(255,5): error TS2411: Property 'titleLocArgs' of type 'string | undefined' is not assignable to string index type 'string'.

Looking at the type in question, 

  type NotificationMessagePayload = {

    tag?: string;

    body?: string;

    icon?: string;

    badge?: string;

    color?: string;

    sound?: string;

    title?: string;

    bodyLocKey?: string;

    bodyLocArgs?: string;

    clickAction?: string;

    titleLocKey?: string;

    titleLocArgs?: string;

    [other: string]: string;

  };

The problem seems to be that with strict null checking enabled, and a string indexer defined, the string indexer forces the named properties to only have string types.  Disabling strict null checks OR removing the indexer corrects the issue.

I think the intended semantics here are to allow indexing of arbitrary string properties, but also provide a set of known-good fields.  If that is the case, then the named fields should be changed from optional to required.

[Jacob Wenger]

Hey Chet,

Sorry for not responding to your earlier. I was really confused as to why this wasn't working, but I finally figured it out!

The intention is indeed to indicate that there is a set of known fields (to help with autocomplete) and you can optionally pass any extra fields if so desired (as noted by the [other: string]) . But each of the known fields is optional, so the ? is actually important. The issue with the type comes with the [other: string]: string; line. That makes TypeScript think that all keys are of type string, even though we've indicated some should be optionally string (or, as TypeScript sees it, string | undefined). So, the fix is to simply change that last line to [other: string]: string | undefined;.

I'll get this type (and others like it) updated in our next release so that it will work out-of-the-box with strictNullChecks.

Cheers,
Jacob

--
You received this message because you are subscribed to the Google Groups "Firebase Google Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firebase-talk+unsubscribe@googlegroups.com.
To post to this group, send email to fireba...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/259e9e93-a93a-460c-96fd-d72bf0fcfd5b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Jacob Wenger]

Hey Chet,

A fix for this went out with yesterday's 4.1.3 release of the Admin Node.js SDK. Thanks again for raising this interesting issue! I learned something new about TypeScript while trying to fix it :)

Let me know if you come across any more issues, TypeScript or otherwise.

Cheers,

Jacob