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

Trigger support for withConverter #872

Open
larssn opened this issue Mar 15, 2021 · 8 comments
Open

Trigger support for withConverter #872

larssn opened this issue Mar 15, 2021 · 8 comments

Comments

@larssn
Copy link

larssn commented Mar 15, 2021

Not sure if you accept Feature Requests, but I'll make it brief:

Would be nice if the onCreate, onWrite, onUpdate and onDelete triggers had support for withConverter:

Example:

firestore
  .document(`businesses/{businessId}`)
  .withConverter(new BusinessConverter())
  .onWrite(async (change, context) => {
    const business: Business = change.after.data()
  })

Thanks

ref: googleapis/nodejs-firestore#1449

@google-oss-bot
Copy link
Collaborator

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

@inlined
Copy link
Member

inlined commented Mar 15, 2021

Great idea! I don't have a timeline for this, but I'll keep it in mind for our roadmap.

@rconjoe
Copy link

rconjoe commented Sep 16, 2021

Second this! I ended up here wondering if this was already a thing. We have some model classes whose methods are called immediately inside an onUpdate. It would save a few lines in every trigger to be able to have those models converted on the way out of Firestore like we do in our database classes.

Thanks for all the great work, my team and I love using Firebase 🙂

@dfdgsdfg
Copy link

dfdgsdfg commented Dec 7, 2021

Work around at this moment.

async (snap, context) => {
  const _ref = snap.ref.withConverter(Model.converter)
  const _snap = await _ref.get()
  const model = _snap.data()
}

@larssn
Copy link
Author

larssn commented Feb 8, 2022

Work around at this moment.

async (snap, context) => {
  const _ref = snap.ref.withConverter(Model.converter)
  const _snap = await _ref.get()
  const action = _snap.data()
}

That would cause an additional read, I'm afraid.

@inlined
Copy link
Member

inlined commented Feb 9, 2022

I'll admit I'm not used to using converters. It seems like a reasonable workaround would be

async (snap, context) => {
  const action = Model.converter.fromFirestore(snap.data())
}

But a converter expects a QueryDocumentSnapshot and we return a DocumentSnapshot. CC @schmidt-sebastian in case he has ideas what to do about the type mismatch.

@schmidt-sebastian
Copy link

QueryDocumentSnapshots are just DoumentSnapshots for documents that are known to exists. Code like data = snapshot.exists ? fromFirestore(snapshot as QueryDocumentSnapshot) : null is perfectly safe.

@charlesfries
Copy link

Anything I can do to help get this moved along? This would be a great feature.

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

7 participants