flamelink / flamelink-js-sdk

🦊 Official Flamelink JavaScript SDK for both the Firebase Realtime database and Cloud Firestore
https://flamelink.github.io/flamelink-js-sdk
MIT License
43 stars 5 forks source link

Flamelink 1.0.0-alpha.33: "Cannot read property 'delete' of undefined" when calling update #147

Closed zinhart closed 3 years ago

zinhart commented 3 years ago

Hello Flamelink Dev Team

As the title suggests,
I'm currently using the new sdk which supports the cloud firestore. I was using the sdk in a firebase cloud function in a test app when I ran into something strange. Calling app.content.update(...) fails with a "Cannot read property 'delete' of undefined."

After some digging I found that in /directories-relevant-to-my-machine/my-project-root/functions/node_modules/@flamelink/sdk-content-cf/dist/cjs/index.js on line 2364(specifically lines 2362-2366) there is:

else {
      if (typeof payload === 'object') {
           payload['_fl_meta_.createdFromLocale'] = context.firebaseApp.firestore.FieldValue.delete();
      }
  }

So it seems that FieldValue is undefined. Please keep in mind this is in the context of Flamelink 1.0.0-alpha.33.
What's interesting in that in version 1.0.0-alpha.30 in the same file this entire else block is missing.

Just to be clear 1.0.0-alpha.30 (lines: 2355-2363)

                        case 1:
                            snapshot = _b.sent();
                            if (snapshot.empty) {
                                sdkUtils.logWarning("No entry existed for schema \"" + schemaKey + "\" with ID \"" + entryId + "\" - creating new entry instead.");
                                return [2, api.add({ schemaKey: schemaKey, entryId: entryId, data: data })];
                            }
                            content = [];
                            snapshot.forEach(function (doc) { return content.push(doc); });
                            return [4, content[0].ref.update(payload)];

while in 1.0.0-alpha.33 (lines: 2356-2369)

                        case 1:
                            snapshot = _b.sent();
                            if (snapshot.empty) {
                                sdkUtils.logWarning("No entry existed for schema \"" + schemaKey + "\" with ID \"" + entryId + "\" - creating new entry instead.");
                                return [2, api.add({ schemaKey: schemaKey, entryId: entryId, data: data, env: env, locale: locale })];
                            }
                            else {
                                if (typeof payload === 'object') {
                                    payload['_fl_meta_.createdFromLocale'] = context.firebaseApp.firestore.FieldValue.delete();
                                }
                            }
                            content = [];
                            snapshot.forEach(function (doc) { return content.push(doc); });
                            return [4, content[0].ref.update(payload)];

I didn't have time to check what commits the changes correlate to but hopefully I've done enough hand waving.

I discovered this change only because I accidentally deployed deployed my cloud functions without saving flamelink as a dependency and installed version 1.0.0-alpha.33 of the sdk (I had 1.0.0-alpha.30 installed in the project root).

Also as a side note why does app.content.update(...) with an entryId that does not exist create a new record instead of just failing? I expect trying to update something that doesn't exist yet to fail instead of creating a new record. If I wanted to create a new record I would just call app.content.add(...).

Btw Flamelink is awesome!

gitdubz commented 3 years ago

Hi @zinhart

Which version of the Firebase admin SDK are you using within your cloud function? I see there is an issue related to this which I will have a look at https://stackoverflow.com/questions/58186235/fieldvalue-is-undefined

There should not be a problem on the client side if you are using the latest version of the Firebase Client SDK when using 1.0.0-alpha.33

Also as a side note why does app.content.update(...) with an entryId that does not exist create a new record instead of just failing? I expect trying to update something that doesn't exist yet to fail instead of creating a new record. If I wanted to create a new record I would just call app.content.add(...).

We added it for convenience, so you can always use update and not worry about needing to check if there is an entry id use add else use update.

We should probably split this into app.content.[upsert,add,update] in future

zinhart commented 3 years ago

I'm using firebase-admin version 9.2.0.

zinhart commented 3 years ago

We added it a convenience, so you can always use update and not worry about needing to check if there is an entry id use add else use update. We should probably split this into app.content.[upsert,add,update] in future

It would be useful if they were split in the future. Let's say you have an route like: /api/setProductDescription/:product_id and because your are using firebase emulators you make a post request to the following url: http://localhost:5001/firebase-project-id/us-central1/api/setProductDescription/21?description=donuts and that there is no product with id 21. The admin sdk update:

    let id= req.params.product_id;
    let description = req.query.description;
    let product = await admin.firestore().collection('fl_content').doc(id);
    let result = await product.update({
      productDescription: description
    }).then((queryResult) => {
      return {status:200, msg: "success"};
    }).catch((error) => {
      return {status:500, msg: error.message};
    });
    res.status(result.status).send(result.msg);

returns: 5 NOT_FOUND: no entity to update: app: "dev~flamelinktest-9fcbf" path < Element { type: "fl_content" name: "21" } >

gitdubz commented 3 years ago

Hi @zinhart, a new version has been published, you can upgrade to 1.0.0-alpha.34 which should fix the issue for you. Let me know if this fixes the issue for you 👍

zinhart commented 3 years ago

Hello, @gitdubz

1.0.0-alpha.34 has fixed this issue for me.

Thank you