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.

Sign up for GitHub

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

Jump to bottom

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
@thebrianchen
Labels
api: firestore v9

Comments

@gustavohenke
Copy link

gustavohenke commented Oct 23, 2021 •
edited
Loading

[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
@jbalidiong jbalidiong added the v9 label Oct 25, 2021
@thebrianchen thebrianchen self-assigned this Oct 25, 2021
@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 •
edited
Loading

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.

@thebrianchen thebrianchen mentioned this issue Oct 28, 2021
Merged
@google-oss-bot google-oss-bot mentioned this issue Nov 8, 2021
Merged
@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.
Assignees

@thebrianchen thebrianchen

Labels
api: firestore v9
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow type T in WithFieldValue<T> firebase/firebase-js-sdk
4 participants
@gustavohenke @thebrianchen @google-oss-bot @jbalidiong