-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix FirebaseServerApp Typescript exactOptionalPropertyTypes error #8341
Conversation
🦋 Changeset detectedLatest commit: ad7a9ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Report 1Affected ProductsNo changes between base commit (ed1c993) and merge commit (0b145e1).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (ed1c993) and merge commit (0b145e1).Test Logs |
Changeset File Check ✅
|
// | ||
// @public | ||
export function isWebWorker(): boolean; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently I missed the doc generate step on the user-created PR that added this function (#8315). My bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text strings LGTM, thanks!
) This change fixes a TypeScript compilation error. The `FirebaseServerAppSettings.name` field inherited from `FirebaseAppSettings` but the type was redefined from `string` to `undefined`. This redefinition would cause a TypeScript compliation error if `exactOptionalPropertyTypes` was set `true` in `packages/app/tsconfig.json`. This change now uses `omit< , >` to strip the `name` field from the `FirebaseServerAppSettings` declaration, where `FirebaseAppSettings` is extended. Fixes #8336
Discussion
This change fixes a TypeScript compilation error.
The
FirebaseServerAppSettings.name
field inherited fromFirebaseAppSettings
but the type was redefined fromstring
toundefined
. This redefinition would cause a TypeScript compliation error ifexactOptionalPropertyTypes
was settrue
inpackages/app/tsconfig.json
.This change now uses
omit< , >
to strip thename
field from theFirebaseServerAppSettings
declaration, whereFirebaseAppSettings
is extended.Fixes #8336
Testing
exactOptionalPropertyTypes
setting defined astrue
.API Changes
While the type definition of
FirebaseServerAppSettings
has changed visually, the effective signature is the same. In both versions ofFirebaseServerAppSettings
the use of thename
field would result in TypeScript compliation errors. I ensured this by creating a simple TypeScript app that attempted to use thename
field, and compiled against the two varyingFirebaseServerAppSettings
definitiions.