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

TypeScript error when using UpdateData<T> with composite interfaces referencing other interfaces #8656

Open
BenJackGill opened this issue Nov 27, 2024 · 5 comments

Comments

@BenJackGill
Copy link

BenJackGill commented Nov 27, 2024

Operating System

macOS Sequoia 15.1.1

Environment (if applicable)

node

Firebase SDK Version

11.0.2

Firebase SDK Product(s)

Firestore

Project Tooling

Vue 3 with Vite

Detailed Problem Description

TypeScript throws an error when we use UpdateData<T> and T is an interface that is a combination of other interfaces.

This example works because we only have a single User interface. It works because the User interface does not reference any other interfaces:

interface User {
  age: number;
  emotion: {
    happy: boolean;
    sad: boolean;
  };
  name: string;
}

const userUpdate: UpdateData<User> = {
  age: 30,
  "emotion.happy": true,
  name: "Alice",
};

const userDocRef = doc(collection(db, "users", "alice"));

await updateDoc(userDocRef, userUpdate);

This example does not work because we have multiple interfaces that reference each other. Here the User interface is referencing the Emotions interface, causing the error to show:

interface Emotions {
  happy: boolean;
  sad: boolean;
}

interface User {
  age: number;
  emotion: Emotions; // <-- Notice this change!
  name: string;
}

const userUpdate: UpdateData<User> = {
  age: 30,
  "emotion.happy": true, // <--- Error shows here
  name: "Alice",
};

const userDocRef = doc(collection(db, "users", "alice"));

await updateDoc(userDocRef, userUpdate);

This is the TypeScript error we see:

Object literal may only specify known properties, and '"emotion.happy"' does not exist in type '{ age?: number | FieldValue | undefined; emotion?: FieldValue | { happy?: boolean | FieldValue | undefined; sad?: boolean | FieldValue | undefined; } | undefined; name?: string | ... 1 more ... | undefined; }'.ts(2353)

Since types are often built as a composite of other types, it would be good to fix this.

Steps and code to reproduce issue

Use the code snippets above to see the errors.

@BenJackGill BenJackGill added new A new issue that hasn't be categoirzed as question, bug or feature request question labels Nov 27, 2024
@BenJackGill BenJackGill changed the title TypeScript error when using UpdateData<T> with composite interfaces referencing other interfaces TypeScript error when using UpdateData<T> with composite interfaces referencing other interfaces Nov 27, 2024
@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@jbalidiong jbalidiong added api: firestore needs-attention and removed needs-triage new A new issue that hasn't be categoirzed as question, bug or feature request labels Nov 27, 2024
@DellaBitta
Copy link
Contributor

Hi @BenJackGill,

This seems to be a general syntax error outside the bounds of Firestore.

Taking Firestore out of it, this also fails to compile with TypeScript:

const user : User = {
      age: 30,
      "emotion.happy": true, // <--- Error shows here
      name: "Alice",
     };

However, these three options don't throw compliation errors:

const emotion : Emotions = {
  happy: true,
  sad: false
};
    
const user : User = {
  age: 30,
  emotion,
  name: "Alice",
};
     
const userUpdate: UpdateData<User> = {
  age: 30,
   emotion,
   name: "Alice",
};

const userUpdate2: UpdateData<User> = {
  age: 30,
  emotion: {
    happy: true
  },
  name: "Alice"
};

Hope this helps!

@BenJackGill
Copy link
Author

Hi @BenJackGill,

This seems to be a general syntax error outside the bounds of Firestore.

Taking Firestore out of it, this also fails to compile with TypeScript:

const user : User = {
      age: 30,
      "emotion.happy": true, // <--- Error shows here
      name: "Alice",
     };

However, these three options don't throw compliation errors:

const emotion : Emotions = {
  happy: true,
  sad: false
};
    
const user : User = {
  age: 30,
  emotion,
  name: "Alice",
};
     
const userUpdate: UpdateData<User> = {
  age: 30,
   emotion,
   name: "Alice",
};

const userUpdate2: UpdateData<User> = {
  age: 30,
  emotion: {
    happy: true
  },
  name: "Alice"
};

Hope this helps!

The UpdateData<T> type is specifically designed for use with updateDoc to facilitate updates using dot notation. Its purpose is to transform standard types, like User, into a form that supports dot notation, which is sometimes necessary when using updateDoc.

In your first example, I would expect a failure because the User interface does not inherently include a "emotion.happy" property. However, when using UpdateData<User>, this should be permissible because UpdateData<User> is intended to adapt User into a type that accommodates dot notation.

The issue I have is unrelated to that. It arises when interfaces reference each other; this composite structure seems to break the intended functionality of UpdateData<T>.

@MarkDuckworth
Copy link
Contributor

@BenJackGill, thanks for the detailed report. At this point, it's unclear if/when we will be able to change this behavior.

In the meantime, if you can define User and Emotion as types rather than interfaces, then UpdateData<User> should work for you.

type Emotions = {
  happy: boolean;
  sad: boolean;
}

type User = {
  age: number;
  emotion: Emotions;
  name: string;
}

const userUpdate: UpdateData<User> = {
  age: 30,
  "emotion.happy": true,
  name: "Alice",
};

Let me know if that workaround works for you. Thanks.

@MarkDuckworth
Copy link
Contributor

Note to Firestore devs: this may be addressed by modifying the conditional types from T extends Record<string, unknown> to T extends Record<string, any>, but we will also need to assert that this does not adversely affect types with nested arrays. This work should be bundled with other changes to UpdateData.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants