Undefined fields in Firelord

[tcaputi]

I wanted to try Firelord / FirelordJS for my Next.js project. I have a type named UserProfile:

export type UserProfile = {
  tag: string;
  searchTag: string;
  id: string;
  name: string;
  photoUrl?: string;
  bio: string;
};

I then turned it into a Firelord metatype:

export type FbUserProfile = MetaTypeCreator<UserProfile, "profiles">;

And finally I tried to write a doc to my database. It seems that all fields that may be undefined in the original type are converted to being required. Is there a way around this? I found the docs on PossiblyReadAsUndefined but that seems to cover a different use case.

[tylim88]

PossiblyReadAsUndefined only allow you to read as undefined

All type will be converted to required because undefined is not a valid type of Firestore(same as RTDB), which mean it is impossible to write undefined to firestore

source: https://firebase.google.com/docs/firestore/manage-data/data-types

if you try to write undefined to firestore, firestore will throw in runtime, potentially crashing your app

few solutions:

1. ✅check whether photoUrl is string, proceed to write if string; else handle photoUrl is undefined case, this is recommended

if(input.photoUrl){
   // proceed to write...
   // starting from typescript version 4.6 or 4.7 or 4.8 (I cant remember),  
   // TS aware that input.photoUrl is not undefined in this scope
   // which mean you dont need to assign extra variable
}else{
  // handle undefined case
}

2. ❌❌use non-null assertion operator !

const newProfile:UserProfile={
// ... other props
photoUrl: input.photoUrl! // typescript will think this is string
}

this is not a recommended practice because it is lazy and unsafe

3. ✅❌union with null, null is a valid type

type UserProfile = {
  tag: string;
  searchTag: string;
  id: string;
  name: string;
  photoUrl: string | null; // string or null
  bio: string;
};

const newProfile:UserProfile={
// ... other props
photoUrl: input.photoUrl || null
}

but null is not a recommended default value, because it will mess up your query, read here https://stackoverflow.com/questions/71171678/firestore-should-we-avoid-using-value-type-null/71173190#71173190

4. ✅✅use empty string as default value

const newProfile:UserProfile={
// ... other props
photoUrl: input.photoUrl || ""
}

this is the solution which I think is the best

It is simple, you don't need to handle the undefined case, and it doesn't change your type

[tcaputi]

Thanks for the quick response!

This all makes sense but I've been using in my project so this kinda breaks my existing code that I was trying to convert.

firebaseDb.settings({ ignoreUndefinedProperties: true });

[tylim88]

just use the 4th solution, it will not change anything

[tylim88]

the set operation requires all member to be present

if you want set operation that doesnt require all member to be present, use set merge

[tcaputi]

Hmmm... alright. I think some of those will work although its a little annoying. Can I ask why (conceptually) set requires all members to be present? If its type is string | undefined why cant it just use that as the type?

[tcaputi]

Thanks again for your very complete response and all the work and thought you've put into this. But if I may, I think you were missing my original point. Unless I'm missing something, the things you described above are true regardless of whether I have defined a partial field or not.

Lets say I have the field photoUrl?: string in my object and I upload an object without that field. When I fetch the object again it is still valid. If I do an update and add (or don't add) that field, my object is still valid. For set merge you still have all the same problems you described above (which I acknowledge is a tricky problem). But this problem is not made better or worse by having a possibly undefined type.

You are also correct that the user can do whatever they want through an app that isn't using firelord (such as the dev console) and it will totally mess all the type guarantees up. However, the same can happen if the user changes their typing at some point down the line. But this can happen anytime in typescript when data leaves the application (such as through an api call). That does not make typescript useless, it just lowers the places where type errors exist.

TL;DR: I still think it should be possible to support possibly undefined values in firelord.

[tylim88]

I see, you want set to allow specific member to be optional, unlike set merge that allow any member to be optional

First, there is something to keep in mind about how firestore work: the existence of the field affect query

for example, if you try to search for users that don't have photoUrl, and if the photoUrl field does not exist, it is impossible to query such users

If photoUrl exist, then it is possible to query users that dont have photoUrl, either the value is null or empty string or any value that is decided by you to represent "no photo url"

I want set to enforce common best practice and I do not see the benefit for set to accept partial members

It can be done but I need more justifications

[tcaputi]

I understand this limitation, but for my use case it is useful. I don't ever intend on looking up my users based on whether or not they have a photoUrl. Not all fields in an object need to be queryable. Even without querying, it is useful to be able to store fields that may / may not be present. To me, this is one of the only advantages of document based data stores; my data does not need as much structure.

Of course, I can work around this (such as a with an empty string), but then everywhere I create a object I want to store I need to add dummy / placeholder values which is inconvenient.

I don't think storing undefined values is necessarily against best practices as long as I accept the limitations on querying. Again, this is just my opinion though. Thanks again for the response.

[tylim88]

I will add this in the future but I cannot promise when

It may not seem so, but it actually involves a lot of works because of complexity

[tcaputi]

I understand. Thanks for hearing me out. Sorry for creating more work for you. The typing guarantees this library provides are incredibly useful to have.

[tcaputi]

Also, thank you for all the work on this. I am trying to convert as much of my code to strict typing as I can and this library is a big step.

[tcaputi]

I'm on discord as well, if that would be an easier place to have this discussion.

[tylim88]

you can @acid coder in off-topic if you have questions

[tylim88]

if(input.photoUrl){ 
  const newProfile:UserProfile={ // all members are present
  // ... other props
  photoUrl: input.photoUrl
  }
  batch.set(profileCol.doc(newProfile.id),newProfile)
}else{
  // handle undefined case
  const newProfile:UserProfile={ // not all members are present
  // ... all props except photoUrl
  }
  batch.set(profileCol.doc(newProfile.id), newProfile, { merge : true })
}

when merge is true, set will accept data with incomplete member

[tylim88]

keep in mind that if you using set merge, there is no way firelord know what member is not existing

so in your case you need to union photoUrl with possiblyReadAsUndefined

to understand the purpose of possiblyReadAsUndefined, read https://firelordjs.com/guides/possibly_read_as_undefined

[tcaputi]

Yeah that's what I figured and I read about PossiblyReadAsUndefined. I think I can cobble something together with the solutions you gave me.