parse-server
parse-server copied to clipboard
feat: add `ParseQuery.triggerFields` to trigger LiveQuery only on update of specific fields
New Pull Request Checklist
- [x] I am not disclosing a vulnerability.
- [x] I am creating this PR in reference to an issue.
Issue Description
Live Query fires when fields are not updated, even when using the fields option. This results in unnecessary overhead.
Related issue: #8027
Approach
Checks if fields have changed, if not do not fire LQ.
TODOs before merging
- [x] Add tests
- [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)
Thanks for opening this pull request!
- ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as
Closes: #123in the PR description, so I can recognize it.
Codecov Report
Base: 94.34% // Head: 93.88% // Decreases project coverage by -0.45% :warning:
Coverage data is based on head (
8aade83) compared to base (40810b4). Patch coverage: 100.00% of modified lines in pull request are covered.
:exclamation: Current head 8aade83 differs from pull request most recent head 0ae3b3d. Consider uploading reports for the commit 0ae3b3d to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## alpha #8028 +/- ##
==========================================
- Coverage 94.34% 93.88% -0.46%
==========================================
Files 181 181
Lines 14388 14401 +13
==========================================
- Hits 13574 13521 -53
- Misses 814 880 +66
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/LiveQuery/ParseLiveQueryServer.js | 95.75% <100.00%> (-0.10%) |
:arrow_down: |
| src/Adapters/Cache/RedisCacheAdapter.js | 17.39% <0.00%> (-73.92%) |
:arrow_down: |
| src/LiveQuery/ParseCloudCodePublisher.js | 84.21% <0.00%> (-15.79%) |
:arrow_down: |
| src/Routers/AggregateRouter.js | 90.00% <0.00%> (-8.00%) |
:arrow_down: |
| src/ParseServerRESTController.js | 92.42% <0.00%> (-4.55%) |
:arrow_down: |
| src/batch.js | 89.47% <0.00%> (-3.51%) |
:arrow_down: |
| src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 89.80% <0.00%> (-2.34%) |
:arrow_down: |
| src/RestWrite.js | 94.46% <0.00%> (-0.45%) |
:arrow_down: |
| src/ParseServer.js | 92.20% <0.00%> (-0.44%) |
:arrow_down: |
| src/Controllers/DatabaseController.js | 93.80% <0.00%> (-0.29%) |
:arrow_down: |
| ... and 3 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Could you please rename a fix describing what is broken, instead of how it should behave?
I guess here it would be something like:
fix: setting the
fieldsparameter still triggers LiveQuery update events for unrelated fields
I will reformat the title to use the proper commit message syntax.
I will reformat the title to use the proper commit message syntax.
I see a lot of the tests passing; is this ready for review yet?
@dblythy If I read this correctly, this is a breaking change; could you please suggest a note for the changelog that describes the full scope of change in this PR? Then we should be good to merge.
The label type:feature cannot be used here.
The label block:major cannot be used here.
The label type:feature cannot be used here.
This isn't breaking and does not change the fields behavour (although perhaps fields should be renamed to select for consistency).
fields: behaves as select, defines what fields to return
listen: only fires LQ if the props have been mutated
I think we can
- add the
selectmethod with the behavior of the currentfields - deprecate the
fieldsmethod
Since we're preparing a major release we could reduce the work and just rename fields to select, which makes this a breaking change. But what does that mean for the LiveQuery client SDKs? The SDKs are client side and if a breaking change requires to update clients (which usually takes more time) we don't want people be stuck on Parse Server 5 because of that. So I'd suggest to only deprecate fields.
Yes, I agree. The issue of renaming .fields to .select and deprecating .select is a different issue to this PR though - this PR introduces a new feature listen that allows LiveQuery to conform to the spec:
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]
Although query.fields will need to updated to query.listen
Got it, so we can consider this as PR as non-breaking and ready for merge?
And the current behavior of fields is purely like select, there is no overlap with the new listen, correct?
Yep! We will just need to update the spec accordingly - describe fields as restricting fields, and listen to the current explanation for fields
I know it's a last-minute question, but wouldn't the term watch make more sense than listen?
- "listen" may imply a passive unspecific observation, like listening on a port for any signal to come in
- "watch" may imply an active specific observation, like watching a Parse Object property for changes
What do you think?
🎉 This change has been released in version 6.0.0-alpha.25
🎉 This change has been released in version 6.0.0-beta.1
🎉 This change has been released in version 6.0.0