googleapis / nodejs-firestore

Node.js client for Google Cloud Firestore: a NoSQL document database built for automatic scaling, high performance, and ease of application development.
https://cloud.google.com/firestore/
Apache License 2.0
640 stars 149 forks source link

Updating a nested document with a dynamic path using typescript #1890

Open PWhiddy opened 1 year ago

PWhiddy commented 1 year ago

I have some code which was working great with earlier versions of firestore/typescript, but after updating can't see how firestore expects nested objects to be updated with dynamic paths.

It happily accepts a single string literal like this:

// no type error
const keyPath = "paymentHistory.test";
t.update(userRef, {
  [keyPath]: payment,
});

But strangely this results in a complex type error involving AddPrefixToKeys.

// type error
const keyPath = "paymentHistory." + "test";
t.update(userRef, {
  [keyPath]: payment,
});

The paymentHistory property on the User object is defined like this:

paymentHistory: Record<string, Payment>

I am using package versions:

"firebase-admin": "11.10.1",
"firebase-functions": "4.4.1",
"typescript": "5.2.2"

How can I satisfy the types, when the nested key "test" is a variable and not known at build time?

Thanks!

milaGGL commented 1 year ago

Hi @PWhiddy, thank you for reporting this issue. Could you please provide a full repro of what you are updating, ie, what is the type of t, and userRef? I am suspecting this might be related to this https://github.com/firebase/firebase-js-sdk/issues/5853

PWhiddy commented 1 year ago

Hi! I'm not sure if it's related to that issue. I made a minimal example that fully reproduces this:
src/index.ts

import {firestore} from "firebase-admin";
import {CollectionReference} from "firebase-admin/firestore";

interface MyDoc {
  nestedA: Record<string, number>
  // The issue goes away if nestedB is removed
  nestedB: Record<string, string>
}

const fstore = firestore();

// The type error goes away if the collection
// is not asserted as "MyDoc", but then type safety is lost
const docStore = fstore.collection("all_docs") as CollectionReference<MyDoc>;

const docRef = docStore.doc("test_doc");

const goodKey = "nestedA.test";
const badKey  = "nestedA." + "test";

// works fine
fstore.runTransaction(async (t) => {
  t.update(docRef, {
    [goodKey]: 3,
  });
});

// type error
fstore.runTransaction(async (t) => {
  t.update(docRef, {
    [badKey]: 3,
  });
});

package.json

{
  "name": "test",
  "version": "0.0.1",
  "scripts": {
    "build": "tsc"
  },
  "main": "index.ts",
  "dependencies": {
    "firebase-admin": "^11.10.1"
  },
  "devDependencies": {
    "typescript": "^5.2.2"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "module": "commonjs",
    "outDir": "lib",
    "strict": true,
    "target": "ES2022",
  },
  "include": [
    "src/**/*"
  ],
  "rootDirs": ["./src"]
}

Really I need the nested key "test" to be dynamic, as is allowed by the type definition of MyDoc, but this simpler case of concatenated string literals makes the strangeness more obvious. This code built fine using an earlier version of typescript/firestore from late last year.

Here is a running demo of these files:
https://codesandbox.io/p/sandbox/gallant-banzai-ypz333 repro

milaGGL commented 1 year ago

Hi @PWhiddy, we have recently fixed a similar issue in the firebase-js-sdk: https://github.com/firebase/firebase-js-sdk/pull/7318. We are planning to port the changes to nodejs-firestore, but I cannot provide an estimated timeline for it. This is added to our backlog along with other tasks. Rest assured, I'll keep you posted on any updates to this thread.

Google internal users please see b/298394991.

ehsannas commented 9 months ago

Internally tracked by b/311751201.

VictorLeach96 commented 4 months ago

Any update on this? it's stopping us upgrading past v4

Volna13 commented 2 months ago

Any update on this?