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

Suggestions for "nested collections in mixed" #6566

Conversation

kraenhansen
Copy link
Member

What, How & Why?

Since my suggested changes are a bit too involved for GitHub comments, I've created a PR with three separate suggestions.

@kraenhansen kraenhansen force-pushed the kh/nested-collections-in-mixed-suggestions branch from 61f5c4b to d736a2e Compare March 21, 2024 12:50
extends OrderedCollection<
T,
[number, T],
/** @internal */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super strange, but apparently putting this on a single line doesn't trigger the stripping of token following the /** @intenal */ - see https://www.typescriptlang.org/play?stripInternal=true#code/C4TwDgpgBAQghgJwDwBUA0UD0AqbUACAlgHbAQLFwA2U2mUAqgHxQC8UKA3AFDeiSw4ALyTd03HHiKlylGnW7M2HHkA

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay 🤔 Definitely good to know!

Copy link
Contributor

@elle-j elle-j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions Kraen!

@elle-j elle-j merged commit 2a6c9ba into lj/nested-collections-in-mixed Mar 21, 2024
29 of 31 checks passed
@elle-j elle-j deleted the kh/nested-collections-in-mixed-suggestions branch March 21, 2024 13:15
elle-j added a commit that referenced this pull request Apr 2, 2024
…ions (#6513)

* Move geospatial helper functions to related file.

* Implement setting nested lists in Mixed.

* Implement setting nested dictionaries in Mixed.

* Implement getting nested lists in Mixed.

* Implement getting nested dictionaries in Mixed.

* Test creating and accessing nested lists and dicts.

* Make previous flat collections tests use the new 'expect' function.

* Test that max nesting level throws.

* Delegate throwing when using a Set to 'mixedToBinding()'.

* Implement setting nested collections on a dictionary via setter.

* Test nested collections on dictionary via setter.

* Minor update to names of tests.

* Combine nested and flat collections tests into same suite.

* Implement setting nested collections on a list via setter.

* Test nested collections on list via setter.

* Refactor common test logic to helper functions.

* Optimize property setter for hot-path and use friendlier err msg.

* Refactor test helper function to build collections of any depth.

* Implement inserting nested collections on a list via 'push()'.

* Test nested collections on a list via 'push()'.

* Test updating dictionary entry to nested collections via setter.

* Test updating nested list/dictionary item via setter.

* Test removing items from collections via 'remove()'.

* Test object notifications when modifying nested collections.

* Group previous notification tests into one test.

* Group collection notifications tests into 'List' and 'Dictionary'.

* Test collection notifications when modifying nested collections.

* Remove collections from test context.

* Test filtering by query path on nested collections.

* Align object schema property names in tests.

* Test filtering with int at_type.

* Implement setting nested collections on a dictionary via 'set()' overloads.

* Test JS Array method 'values()'.

* Test JS Array method 'entries()'.

* Implement getting nested collections on dictionary 'values()' and 'entries()'.

* Test 'values()' and 'entries()' on dictionary with nested collections.

* Remove unnecessary 'fromBinding()' calls.

* Refactor collection helpers from 'PropertyHelpers' into the respective collection file.

* Introduce list/dict sentinels to circumvent extra Core access.

* Rename getter to default.

* Remove redundant 'snapshotGet'.

* Add abstract 'get' and 'set' to 'OrderedCollection'.

* Rename the collection helpers to 'accessor'.

* Move tests into subsuites.

* Fix 'Results.update()'.

* Support nested collections in 'pop()', 'shift()', 'unshift()', 'splice()'.

* Test list 'pop()'.

* Test list 'shift()'.

* Test list 'unshift()'.

* Test list 'splice()'.

* Return 'not found' for collections searched for in 'indexOf()'.

* Test ordered collection 'indexOf()'.

* Support list/dict sentinels in JSI.

* Test references per access.

* Enable skipped tests after Core bug fix.

* Point to updated Core.

* Fix accessor for non-Mixed top-level collection with Mixed items.

* Enable and fix previously skipped test.

* Update 'mixed{}'.

* Update 'mixed<>'.

* Remove now-invalidated test.

* Remove unused injectable from Node bindgen template.

* Replace if-statements with switch.

* Add explicit Results and Set accessors for Mixed.

* Adapt to change in Core treating missing keys as null in queries.

* Rename insertion function.

* Include tests of Dictionary property type with Mixed.

* Test reassigning to new collection and self-assignment.

* Test mixed

* Update 'mixed[]'.

* Test results accessor.

* Update error messages.

* Make accessor helpers an object field rather than spread.

* Suggestions for "nested collections in mixed" (#6566)

* Fix type bundling issue

* Inline functions into "create*Accessor*" functions

* Refactored typeHelpers out of accessors

* Remove leftover 'Symbol_for' in node-wrapper template.

* Test not invalidating new collection.

* Remove test for max nesting level.

The max nesting level in debug in Core has been updated to be the same as for release.

* Remove reliance on issue-fix in certain tests.

* Add key path test for object listener on mixed field.

* Use '.values()' and '.entries()' in iteration.

* Update comments.

* Add CHANGELOG entry.

---------

Co-authored-by: Kræn Hansen <[email protected]>
elle-j added a commit that referenced this pull request Apr 11, 2024
…ions (#6513)

* Move geospatial helper functions to related file.

* Implement setting nested lists in Mixed.

* Implement setting nested dictionaries in Mixed.

* Implement getting nested lists in Mixed.

* Implement getting nested dictionaries in Mixed.

* Test creating and accessing nested lists and dicts.

* Make previous flat collections tests use the new 'expect' function.

* Test that max nesting level throws.

* Delegate throwing when using a Set to 'mixedToBinding()'.

* Implement setting nested collections on a dictionary via setter.

* Test nested collections on dictionary via setter.

* Minor update to names of tests.

* Combine nested and flat collections tests into same suite.

* Implement setting nested collections on a list via setter.

* Test nested collections on list via setter.

* Refactor common test logic to helper functions.

* Optimize property setter for hot-path and use friendlier err msg.

* Refactor test helper function to build collections of any depth.

* Implement inserting nested collections on a list via 'push()'.

* Test nested collections on a list via 'push()'.

* Test updating dictionary entry to nested collections via setter.

* Test updating nested list/dictionary item via setter.

* Test removing items from collections via 'remove()'.

* Test object notifications when modifying nested collections.

* Group previous notification tests into one test.

* Group collection notifications tests into 'List' and 'Dictionary'.

* Test collection notifications when modifying nested collections.

* Remove collections from test context.

* Test filtering by query path on nested collections.

* Align object schema property names in tests.

* Test filtering with int at_type.

* Implement setting nested collections on a dictionary via 'set()' overloads.

* Test JS Array method 'values()'.

* Test JS Array method 'entries()'.

* Implement getting nested collections on dictionary 'values()' and 'entries()'.

* Test 'values()' and 'entries()' on dictionary with nested collections.

* Remove unnecessary 'fromBinding()' calls.

* Refactor collection helpers from 'PropertyHelpers' into the respective collection file.

* Introduce list/dict sentinels to circumvent extra Core access.

* Rename getter to default.

* Remove redundant 'snapshotGet'.

* Add abstract 'get' and 'set' to 'OrderedCollection'.

* Rename the collection helpers to 'accessor'.

* Move tests into subsuites.

* Fix 'Results.update()'.

* Support nested collections in 'pop()', 'shift()', 'unshift()', 'splice()'.

* Test list 'pop()'.

* Test list 'shift()'.

* Test list 'unshift()'.

* Test list 'splice()'.

* Return 'not found' for collections searched for in 'indexOf()'.

* Test ordered collection 'indexOf()'.

* Support list/dict sentinels in JSI.

* Test references per access.

* Enable skipped tests after Core bug fix.

* Point to updated Core.

* Fix accessor for non-Mixed top-level collection with Mixed items.

* Enable and fix previously skipped test.

* Update 'mixed{}'.

* Update 'mixed<>'.

* Remove now-invalidated test.

* Remove unused injectable from Node bindgen template.

* Replace if-statements with switch.

* Add explicit Results and Set accessors for Mixed.

* Adapt to change in Core treating missing keys as null in queries.

* Rename insertion function.

* Include tests of Dictionary property type with Mixed.

* Test reassigning to new collection and self-assignment.

* Test mixed

* Update 'mixed[]'.

* Test results accessor.

* Update error messages.

* Make accessor helpers an object field rather than spread.

* Suggestions for "nested collections in mixed" (#6566)

* Fix type bundling issue

* Inline functions into "create*Accessor*" functions

* Refactored typeHelpers out of accessors

* Remove leftover 'Symbol_for' in node-wrapper template.

* Test not invalidating new collection.

* Remove test for max nesting level.

The max nesting level in debug in Core has been updated to be the same as for release.

* Remove reliance on issue-fix in certain tests.

* Add key path test for object listener on mixed field.

* Use '.values()' and '.entries()' in iteration.

* Update comments.

* Add CHANGELOG entry.

---------

Co-authored-by: Kræn Hansen <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants