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

.fields() doesn't work on ParseLiveQuery JS #1184

Closed
dblythy opened this issue Jun 8, 2020 · 12 comments
Closed

.fields() doesn't work on ParseLiveQuery JS #1184

dblythy opened this issue Jun 8, 2020 · 12 comments

Comments

@dblythy
Copy link
Member

dblythy commented Jun 8, 2020

Not sure if this is an issue with Parse Server, or the JS SDK.

However, ParseLiveQuery server states:

subscriptionInfo.fields = request.query.fields

Yet I could not find any way to set .fields on the request query.

I presume .select is intended for this, however I don't think ParseQuery ever passes .select() to .fields.

My workaround for JS was to set query.fields = [''].

Happy to work on a PR if need be!

@dplewis
Copy link
Member

dplewis commented Jun 11, 2020

It should work as the keys are passed in here

Does your live query return all keys? Can you write a test case?

@dblythy
Copy link
Member Author

dblythy commented Jul 14, 2020

Will do!

@dblythy
Copy link
Member Author

dblythy commented Jul 28, 2020

    const object = new TestObject();
    object.set('baz','qux')
    await object.save();

    const query = new Parse.Query(TestObject);
    query.select('baz')
    
    const subscription = await query.subscribe();
    subscription.on('update', async (newObject, object) => {
      fail("field 'foo' wasn't selected, create shouldn't fired.")
    });
    
    object.set({ foo: 'bar'});
    await object.save();

    setTimeout(() => {
        done();
    }, 1000);

dplewis added a commit that referenced this issue Jul 28, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@dplewis
Copy link
Member

dplewis commented Jul 28, 2020

@dblythy I opened a PR with a test case #1203

It works for me. Let me know if it accurately represents the issue you are having

@dblythy
Copy link
Member Author

dblythy commented Jul 28, 2020

Thank you mate. I might have misunderstood the LiveQuery docs.

Suppose the ParseObject Player contains three fields name, id and age. If you are only interested in the change of the name field, you can set query.fields to [name].

My thinking is that "update" should only fire when "name" is changed (if "fields" is selected).

Is that correct?

@dplewis
Copy link
Member

dplewis commented Jul 28, 2020

Interesting. I actually don't know. I was focused on this part.

when the change of a Player ParseObject fulfills the subscription, only the name field will be sent to the clients instead of the full Player ParseObject

I assumed it would function just like .select.

@parse-community/core-maintainers What should the proper implementation for this be?

@dplewis
Copy link
Member

dplewis commented Jul 28, 2020

@dblythy Oh I see what you were trying to do in your example.

@dblythy
Copy link
Member Author

dblythy commented Jul 28, 2020

In my use case, I am creating a LiveQuery on the _User class, but I only need updates on the "notificationCount" field, which is incremented on an afterSave trigger.

I.e

const query = new Parse.Query(Parse.User);
query.select("notificationCount");

const subscription = await query.subscribe();
subscription.on('update', (object) => {
      this.notificationCount = object.get('notificationCount');
     // if user updates email / firstName, this block fires, however notificationCount hasn't changed so it's not entirely restrictive.
})

I don't want the update event to fire when "firstName" / "email" is updated.

I can easily compare originalObject and newObject but I was thinking fields would be more efficient to implicitly choose which keys you want to listen to changes in, restricting the amount of LQ messages being sent too.

@dblythy
Copy link
Member Author

dblythy commented Jul 29, 2020

@dplewis what do you think about the addition of a few more LQ triggers, beforeCreate and beforeUpdate? I'm thinking that these could be like an afterFind trigger, and maybe throwing from the triggers could be silent and just prevent the event from being called.

I could do something like:

Parse.Cloud.beforeUpdate(Parse.User, request => {
    if (!request.object.dirty('Notifications')) {
        throw "No need to run update event as notifications haven't changed."
    }
})

I can't think of too many use cases where restricting the query using beforeSubscribe wouldn't cover restricting the liveQuery events (other than .dirty), but maybe this way it could provide flexibility for users to decide for themselves.

@dplewis
Copy link
Member

dplewis commented Aug 17, 2020

@parse-community/core-maintainers What do you guys think?

@davimacedo
Copy link
Member

I'd prefer to have an additional option in the current triggers for not broadcasting the change. Or, if stop the broadcasting is the use case, wouldn't be better to improve the current logic to not broadcast when not needed by default?

dplewis added a commit that referenced this issue Aug 18, 2020
@dplewis
Copy link
Member

dplewis commented Oct 19, 2020

Closing via parse-community/parse-server#6859

@dplewis dplewis closed this as completed Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants