Support union types and optional fields with dot separation on UpdateData #5394
Conversation
🦋 Changeset detectedLatest commit: 073ab9c 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 |
Changeset File Check ✅
|
thebrianchen
requested review from
Feiyang1,
hiranya911 and
hsubox76
as code owners
Size Analysis Report |
.changeset/clean-cameras-check.md
Outdated
| '@firebase/firestore': minor | ||
| --- | ||
|
|
||
| Fixed a bug where UpdateData did not recognize optional, dot-separated string fields |
There was a problem hiding this comment.
If this goes straight into release notes, I'd suggest UpdateData as a literal, and a period. Otherwise LGTM, thanks!
|
How do we use updateData in firebase-admin? @thebrianchen |
…sdk into bc/fix-update-data
| AddPrefixToKeys<K, UpdateData<T[K]>> | ||
| : // TypedUpdateData is always a map of values. | ||
| never; | ||
| // Union with `undefined` to allow for optional nested fields |
thebrianchen
force-pushed
the
bc/fix-update-data
branch
from
083ab52 to
160423a
Compare
thebrianchen
changed the title
| @@ -1542,6 +1542,108 @@ describe('withConverter() support', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('supports optional fields', () => { | |||
There was a problem hiding this comment.
Can you add a test for null?
| */ | ||
| export type ChildUpdateFields<T1, K extends string> = | ||
| // Only allow nesting for map values | ||
| T1 extends Record<string, unknown> |
There was a problem hiding this comment.
Is there a better name than T1? Reminds me of TI-89
…sdk into bc/fix-update-data
| * | ||
| * See https://www.typescriptlang.org/docs/handbook/advanced-types.html#distributive-conditional-types | ||
| */ | ||
| export type ChildUpdateFields<K extends string, V> = |
There was a problem hiding this comment.
This looks identical to the initial implementation. What has changed that adds the support for optional fields?
There was a problem hiding this comment.
This is an excerpt from the documentation link:
Conditional types in which the checked type is a naked type parameter are called distributive conditional types. Distributive conditional types are automatically distributed over union types during instantiation. For example, an instantiation of T extends U ? X : Y with the type argument A | B | C for T is resolved as (A extends U ? X : Y) | (B extends U ? X : Y) | (C extends U ? X : Y).
Here's a TS playground link that shows the before and after.
There was a problem hiding this comment.
Thanks for the example! Now it makes sense to me.
Fixes a bug where
UpdateDatawould not recognize optional, dot-separated string fields. See comment for repro details.