parse-community / Parse-SDK-JS

The JavaScript SDK for Parse Platform
https://parseplatform.org
Apache License 2.0
1.32k stars 596 forks source link

feat: Support dot notation on array fields #2120

Closed dplewis closed 3 months ago

dplewis commented 4 months ago

Pull Request

Issue

Using Dot notation on array fields messes up the internal state of objects. Most notably internally they are handled like objects instead of arrays. This would prevent other array operations like add, remove from working. Also this may cause errors on the Server.

{ items: { '0': { count: 20 }, '1': { count: 5 } } }

instead of

{ items: [{ count: 20 }, { count: 5 }] }

Closes: https://github.com/parse-community/Parse-SDK-JS/pull/1343

Approach

Update ObjectStateMutation estimateAttributes and commitServerChanges. estimateAttributes is called whenever obj.attributes is accessed. commitServerChanges is called on obj.save and obj.fetch

TODO

Tasks

parse-github-assistant[bot] commented 4 months ago

Thanks for opening this pull request!

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (b50790a) to head (5c1e04b). Report is 45 commits behind head on alpha.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## alpha #2120 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 61 64 +3 Lines 6186 6364 +178 Branches 1499 1514 +15 ========================================== + Hits 6186 6364 +178 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mortenmo commented 4 months ago

so the syntax is set('arr.1.name', 'Bob');? I'd be curious if the server creates the right mongo update to support this, might be worth a integration test.

Unrelated, I noticed in one of my PRs that ObjectStateMutations.js has a function estimateAttribute that doesn't seem to support dot-notation at all, while the plural estimateAttributes does (which you updated here). I couldn't find any usage of the first one, is it just dead code?

dplewis commented 4 months ago

so the syntax is set('arr.1.name', 'Bob');? I'd be curious if the server creates the right mongo update to support this, might be worth a integration test.

I'm curious as well but https://github.com/parse-community/parse-server/pull/9115 will need to be merged first. I think it should work based on the tests I wrote.

estimateAttribute that doesn't seem to support dot-notation at all

I think this might be dead code, good catch

dplewis commented 3 months ago

@mtrezza This is ready to merge.

parseplatformorg commented 3 months ago

🎉 This change has been released in version 5.2.0-alpha.1

parseplatformorg commented 2 months ago

🎉 This change has been released in version 5.2.0-beta.1

parseplatformorg commented 2 months ago

🎉 This change has been released in version 5.2.0