这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@xperiandri
Copy link
Owner

@TysonMN I started the implementation of incremental loading support.
What do you think of naming and API shape?

@TysonMN
Copy link

TysonMN commented May 2, 2021

Looks interesting, but I don't think I fully understand. Is there a sample that demonstrates this new behavior?

@xperiandri
Copy link
Owner Author

I expect to update some sample tomorrow (one way seq or submodel seq).
But you can get an idea form the second commit.

Now I struggle with the issue dotnet/fsharp#11516
Do you have any ideas?

@xperiandri
Copy link
Owner Author

@TysonMN, the OneWaySeq sample is updated to include ISupportIncrementalLoading

@TysonMN
Copy link

TysonMN commented May 4, 2021

When I try to compile the branch (currently at commit e4df967), there are ~90 compile errors. I was able to avoid them by unloading Elmish.Uno.Tests and Samples.Skia.Wpf.Host.

I was able to run the OneWaySeq sample. I know what you mean now by incremental loading. The idea seems great.

As for the design, I am a bit put off by the use of TaskCompletionSource<_>. However, I don't immediately see a better way to handle this.

Incremental loading is not a top priority for me at the moment, but I don't want to forget about it either, so I just created elmish/Elmish.WPF#384.

@xperiandri
Copy link
Owner Author

xperiandri commented May 4, 2021

WPF requires GTK to be installed as far as I know

@xperiandri xperiandri force-pushed the incremental_load branch 2 times, most recently from d51f144 to e9810db Compare May 16, 2021 22:46
@xperiandri
Copy link
Owner Author

@TysonMN how would you recommend covering this new functionality with tests?

@TysonMN
Copy link

TysonMN commented May 18, 2021

Given my limited knowledge of this feature, I would want to spend more time on the API. I still hope that uint * TaskCompletionSource<uint> -> unit can be replaced with something more idiomatic like uint -> Async<uint>. Then I expect that this improved API would simplify testing.

Is it possible to replace uint * TaskCompletionSource<uint> -> unit with uint -> Async<uint>?

@xperiandri xperiandri force-pushed the incremental_load branch 2 times, most recently from 2adf622 to 7348396 Compare August 24, 2021 14:18
@xperiandri
Copy link
Owner Author

Probably but I don't know how.

| LoadMore (count, tcs) ->
    let intCount = int count
    let builder = m.IncrementalLoadingNumbers.ToBuilder()
    let max = FlatList.last m.IncrementalLoadingNumbers
    for i = max + 1 to max + intCount do
      builder.Add(i)
    tcs.SetResult(count)
    { m with IncrementalLoadingNumbers = builder.ToImmutable() }

If I replace TaskCompletionSource with Async<uint> I can change this into something like:

type Msg =
  | LoadMore of count: uint * complete: (uint -> Async<unit>)
  | LoadedMore of items: FlatList<int>

let asyncLoadItems (complete: uint -> Async<unit>) (items: FlatList<int>) count = async {
  let intCount = int count
  let builder = items.ToBuilder()
  let max = FlatList.last items
  for i = max + 1 to max + intCount do
    builder.Add(i)

  do! complete count
  return LoadedMore <| builder.ToImmutable ()
 }

let update msg m =
  match msg with
  | LoadMore (count, complete) -> m, asyncLoadItems complete m.IncrementalLoadingNumbers count |> Cmd.OfAsync.result
  | LoadedMore items -> { m with IncrementalLoadingNumbers = items }, Cmd.none

Is it better?

@TysonMN
Copy link

TysonMN commented Aug 24, 2021

I think so, but I would need to play with it.

I am making steading progress (something nearly every day) on Elmish.WPF, so I hope that v4 is not far off. Then I can focus on other ideas like this one.

Keep up the good work! :)

@xperiandri
Copy link
Owner Author

@TysonMN I've created a new branch with simplification and necessary explanation comments
https://github.com/xperiandri/Elmish.Uno/tree/incremental_load_simplified
Have a look at the latest commit in it, please

@TysonMN
Copy link

TysonMN commented Sep 18, 2021

(Sorry for the delay)

Overall, it looks better.

Would it be possible to add incremental loading in a single commit? I think that would make it easier for me to follow the changes.

@xperiandri xperiandri force-pushed the incremental_load branch 2 times, most recently from 738d2d2 to c89a7bc Compare October 16, 2021 11:11
xperiandri added a commit that referenced this pull request Jul 22, 2025
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

Successfully merging this pull request may close these issues.

3 participants