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.

Sign up for GitHub

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

Jump to bottom

Fix ko.observable handling in Linq translations #1870

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix ko.observable handling in Linq translations #1870

wants to merge 2 commits into from

Conversation

exyi
Copy link
Member

@exyi exyi commented Oct 16, 2024

  • FirstOrDefault, LastOrDefault, ... should behave like indexers
    • return observable, if the array contains observables, which then needs to be unwrapped
    • this is a regression in 4.3
  • Where, Order, Take, ... return the same structure which the original array has
  • Select is weird as it unwraps all observables in the array, but not recursively. That's why patch adds the JsObjectObservableMap object, which can represent this mid-state (and many other combinations)
    • Indexing after Select was apparently always broken in DotVVM

exyi added a commit that referenced this pull request Oct 16, 2024
@exyi
Hot fix for missing ko.unwrap in Linq Last,First,ElementA translations
695ea4b 
This is a lighterweight alternative to #1870,
which we might not want to merge to 4.3.
This PR does only fixes the 4.3 regression,
the problem with .Select(...).ToArray()[0] is not fixed
@exyi exyi mentioned this pull request Oct 16, 2024
Open
@exyi
Fix ko.observable handling in Linq translations
0c5c7c6 
* FirstOrDefault, LastOrDefault, ... should behave like indexers
    - return observable, if the array contains observables, which then needs to be unwrapped
    - this is a regression in 4.3
* Where, Order, Take, ... return the same structure which the original array has
* Select is weird as it unwraps all observables in the array, but not recursively. That's why patch adds the JsObjectObservableMap object, which can represent this mid-state (and many other combinations)
   - Indexing after Select was apparently always broken in DotVVM
@exyi exyi force-pushed the fix-linq-translations-observables branch 4 times, most recently from bf2e517 to 43e5a18 Compare October 16, 2024 16:24
@exyi exyi force-pushed the fix-linq-translations-observables branch from 43e5a18 to 562c26f Compare October 16, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@exyi