fix: Root tsconfig doesn't include anything useful and all type-tests in the repository are broken

[macksal]

Description

I found that all type tests (type-test.ts) in the repository are currently broken and do not test anything useful. I noticed this via intellisense when installing the repo locally to fix some other types: nearly everything is typed as any.

Analysing the problem

Current tsconfig only includes packages/**/lib/*.d.ts. (Insight: this doesn't include nested folders underneath lib/) For all the packages I looked at, this only includes the index.d.ts but does not include other type declarations in nested folders. For example, the firestore package has a lot of *.d.ts within the firestore/lib/modular/ directory.

In most cases, the lib/index.d.ts file will import types from those nested directories in the module. firestore/lib/index.d.ts imports from firestore/lib/modular/index.d.ts for example. However, because those nested directories are not part of the tsconfig, they are treated as external dependencies. This is an issue because skipLibCheck is enabled. skipLibCheck ignores type errors in any external dependency, and changes the errored types to any. Oops, that includes all types from nested files within our modules!

FInally, packages attempt to import each other's types. For example, all modules have import { ReactNativeFirebase } from '@react-native-firebase/app' at the top. When installed as a dependency from NPM this is OK, as the imported module will be in node_modules and resolved correctly. However, when running tsc against the repo there's no way for the type checker to resolve these as they are not found in node_modules. You can verify this by checking intellisense in vscode after installing this repo, any time that a package's index.d.ts imports from @react-native-firebase/* you will see all the imported types become any.

Solution

I have made these changes:

1. Include all .d.ts files nested within each package's lib directory, not only the root.
2. Add a tsconfig paths definition to allow the modules to resolve each other when running tsc in the repo.

After this change, the type-test.ts in most packages are broken. That's not caused by this change, in fact the tests are already wrong for many years and have just been masked by the skipLibCheck issue (Since #5590 in 2021!!!). For example, most type-tests treat import firebase from '.' as both a default import and as a star import, which is likely incorrect.

The bottom line is: if skipLibCheck is enabled, it is critically important to include all files you actually want to type check within the scope of a tsconfig.json which is used to run tsc. Otherwise they are treated as external deps and aren't checked at all, with all their types silently becoming any when unresolved.

Related issues

Release Summary

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Yes
• My change supports the following platforms;
  • Android
  • iOS
  • Other (macOS, web)
• My change includes tests;
  • e2e tests added or updated in packages/\*\*/e2e
  • jest tests added or updated in packages/\*\*/__tests__
• I have updated TypeScript types that are affected by my change.
• This is a breaking change;
  • Yes
  • No

Test Plan

Does not affect any user-facing code. tsc is not used anywhere to compile code (noEmit is set). Just makes the type tests in the repository actually functional. They should be fixed individually later.

---

Think react-native-firebase is great? Please consider supporting the project with any of the below:

• 👉 Star this repo on GitHub ⭐️
• 👉 Follow React Native Firebase and Invertase on Twitter

[vercel]

@macksal is attempting to deploy a commit to the Invertase Team on Vercel.

A member of the Team first needs to authorize it.

[CLAassistant]

All committers have signed the CLA.

[mikehardy]

Hi there 👋
Reads a bit like a hate-post with rage-bait title but you've got good insights buried in amongst that, they have a lot of value.

We won't be able to merge this unless the tests pass as well or CI will be broken. I suppose we could disable tests.

That said, we're in the midst of converting everything to be written natively in typescript at this exact moment anyway, which I suppose would solve the problem as well but in a different / more thorough way ?

https://github.com/invertase/react-native-firebase/pulls?q=is%3Apr+is%3Aopen+refactor+TypeScript

Worth it to work through type tests after patching up the root tsconfig here or no, what do you think?