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

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

Open
PWhiddy opened this issue Aug 28, 2023 · 8 comments
Open

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

PWhiddy opened this issue Aug 28, 2023 · 8 comments
Assignees
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue.

Comments

@PWhiddy
Copy link

PWhiddy commented Aug 28, 2023

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!

@PWhiddy PWhiddy added priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue. labels Aug 28, 2023
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Aug 28, 2023
@milaGGL
Copy link
Contributor

milaGGL commented Aug 29, 2023

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 firebase/firebase-js-sdk#5853

@PWhiddy
Copy link
Author

PWhiddy commented Aug 29, 2023

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
Copy link
Contributor

milaGGL commented Aug 31, 2023

Hi @PWhiddy, we have recently fixed a similar issue in the firebase-js-sdk: firebase/firebase-js-sdk#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
Copy link
Contributor

Internally tracked by b/311751201.

@VictorLeach96
Copy link

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

@Volna13
Copy link

Volna13 commented Jun 19, 2024

Any update on this?

@davidcruzcs
Copy link

davidcruzcs commented Sep 17, 2024

Hey! Just wanted to know if there's any update on this.

@tim-gaweco-1
Copy link

Can we get an update on this? This is urgent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

8 participants