Missing type paths for TypeScript 4.7 Node16 modules

[andipaetzold]

[REQUIRED] Describe your environment

• Operating System version: all
• Browser version: all
• Firebase SDK version: all
• Firebase Product: all (auth, database, storage, etc)
• TypeScript: 4.7.2

[REQUIRED] Describe the problem

With TypeScript 4.7 a new module was introduced to improve the ES module support: Node16 (https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#esm-nodejs). When using that module in a tsconfig.json and importing from firebase/xxx, an error is thrown because the type declaration cannot be found.

The issue is that only a global typings/types path is defined. With module: Node16 and strict: true, the path to the types must also be added to the exports field in the package.json. (At least this is the only way I got it working).

Steps to reproduce:

1. Create a new project
2. Install TypeScript 4.7.2
3. Add tsconfig.json with module: Node16 and strict: true
4. Create an index.ts where you import from firebase/xxx
5. Run tsc

You will receive an error similar to this:

index.ts:1:28 - error TS7016: Could not find a declaration file for module 'firebase/database'. '/home/andipaetzold/dev/firebase-node16-issue/node_modules/firebase/database/dist/index.cjs.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/firebase` if it exists or add a new declaration (.d.ts) file containing `declare module 'firebase/database';`

1 import type { Query } from "firebase/database";

I've created a repository to easily reproduce the issue: https://github.com/andipaetzold/firebase-node16-issue

Relevant Code:

index.ts

import type { Query } from "firebase/database";

tsconfig.json

{
  "compilerOptions": {
    "module": "Node16",
    "strict": true,
    "skipLibCheck": true
  }
}

Solution

In order to get the repository linked above working, I had to adjust several files in the node_modules folder:

node_modules/firebase/package.json

    "./database": {
      "node": {
        "require": "./database/dist/index.cjs.js",
        "import": "./database/dist/index.mjs"
      },
      "default": "./database/dist/index.esm.js",
+     "types": "./database/dist/database/index.d.ts"
    },

node_modules/@firebase/database/package.json

"exports": {
    ".": {
      "node": {
        "import": "./dist/node-esm/index.node.esm.js",
        "require": "./dist/index.node.cjs.js"
      },
      "esm5": "./dist/index.esm5.js",
      "standalone": "./dist/index.standalone.js",
      "default": "./dist/index.esm2017.js",
+     "types": "./dist/public.d.ts"
    },
    "./package.json": "./package.json"
  },

With skipLibCheck: false in tsconfig.json, similar changes must be done in all indirectly imported firebase packages

After the adjustments, tsc does not throw and compiles successfully.

In order to fix all imports, the types need to be added to all exports.

I started on this locally, but wanted to open this issue first in case I failed using TypeScript or you are working on a fix already. Also, the solution might be added to the use_typings.js script and I didn't want to mess around with your build setup

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[jbalidiong]

Hi @andipaetzold , thanks for the report. I was able to reproduce the behavior using your minimal repro. Let me check what we can do for this issue or bring someone here that can provide more context about it. I’ll update this thread if I have any information to share.

[andipaetzold]

Would it help if I open a PR with my current changes to show what needs to be done to resolve the issue? Alternatively, here is a PR from the Playwright folks that resolves the same problem: microsoft/playwright#14428

About the triage: This issue affects all packages/APIs. Not only database. Database was chosen to reproduce the issue.

[jbalidiong]

Yes, pull requests are definitely welcome! I'll notify an engineer to have this checked. Thank you for your contribution!

[hsubox76]

This solution looks good, I just wanted to double check if there is a chance adding this field will break anyone using older versions of Node or Typescript or different configurations. It doesn't seem like it should but if you can double check that, that would be great. I think you've provided enough info so you don't need to make a demo PR to show the problem, but if you want to make an actual PR to fully fix the problem, that would be great (but totally optional).

[andipaetzold]

I just created a PR (#6307) to resolve this issue in all firebase packages.

[hsubox76]

This will go out in the next release which is currently scheduled for 6/23/2022 but I'm looking into doing an earlier one as that's a long ways off.

[hsubox76]

FYI this has been released in 9.8.3