Skip to content
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

T isn't assignable to WithFieldValue<T> #5661

Closed
gustavohenke opened this issue Oct 23, 2021 · 4 comments · Fixed by #5675
Closed

T isn't assignable to WithFieldValue<T> #5661

gustavohenke opened this issue Oct 23, 2021 · 4 comments · Fixed by #5675
Assignees

Comments

@gustavohenke
Copy link

gustavohenke commented Oct 23, 2021

[REQUIRED] Describe your environment

  • Operating System version: -
  • Browser version: -
  • Firebase SDK version: 9.1.3
  • Firebase Product: Firestore
  • Typescript 4.4.4

[REQUIRED] Describe the problem

Steps to reproduce:

Create a function which takes an argument of type T, and inside it call any firestore functions that receive WithFieldValue or PartialWithFieldValue (e.g. setDoc, addDoc, etc) with that value T.

Result is that TypeScript will say nope:

Argument of type 'T' is not assignable to parameter of type 'WithFieldValue'.

this creates a problem in that every call to these functions require a type cast, which is not ideal.

Relevant Code:

class Bla<T> {
  withFieldValueT(value: WithFieldValue<T>) {
    
  }

  withT(value: T) {
    // doesn't work
    this.withFieldValueT(value);

    // works but has cast
    this.withFieldValueT(value as unknown as WithFieldValue<T>);
  }
}

const bla = new Bla<{ id: string, foo: number }>();
bla.withFieldValueT({ id: '', foo: 123 });
bla.withT({ id: '123', foo: 123 });

TS playground link

@thebrianchen
Copy link

@gustavohenke You'll need to tell the TS transpiler that the T you reference in withT(value: T) extends WithFieldValue:

withT<A extends WithFieldValue<T>>(value: A)

See the TS playground link

I'm going to mark this issue as closed, but feel free to ask any other questions you may have.

@gustavohenke
Copy link
Author

gustavohenke commented Oct 25, 2021

I don't like that I need to expose Firebase types outside of specific code dealing with Firebase, so I'm keen to fix this.

Maybe a definition like below works better?

type WithFieldValue<T> = T | (T extends Primitive ? T : {
  [K in keyof T]: FieldValue | WithFieldValue<T[K]>
});

type PartialWithFieldValue<T> = Partial<WithFieldValue<T>>;

TS playground

This way, WithFieldValue<T> might be the same as T, but you can also replace it with FieldValue if needed.

I can submit a PR if this works.

@thebrianchen thebrianchen reopened this Oct 26, 2021
@thebrianchen
Copy link

@gustavohenke I hadn't considered the need to expose Firebase types with the current implementation. That's definitely something we can consider. I'll get back to you in the coming week.

@gustavohenke
Copy link
Author

Thanks for looking into this deeper!

@firebase firebase locked and limited conversation to collaborators Dec 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants