FIX remove existing items in stream if reset=true

[micdahl]

When using stream(socket, :elements, elements, reset: true), elements that allready existed in the stream were not deleted from the DOM before the new elements are inserted. If the elements have a different order than before, this ordering was not mapped to the DOM.
By removing the filter for already existing elements to delete them all, the given order of the elements is retained.

Fixes #2767, fixes #2826

[Malian]

Thanks @micdahl for tackling this issue.

Unfortunately, it does not seem to fix the bug described in #2826

I upgraded my repo (https://github.com/Malian/stream_reset_bug/tree/fix-micdahl) with your branch, and the behaviour is still the same.

[micdahl]

Hey @Malian, it seems that the javascript file which contains the change (dom_patch.js) is not used automatically. If I inspect your application with my browser's dev tools, I can see, that the changed js is not used. In my local phoenix application I had to change the LiveSocket import from ./assets/js/app.js to use the correct dir. I do not fully understand the asset pipeline yet, so I am not sure, how to archive that. I think, the javascript inside of the ./deps/phoenix_live_view/assets/js dir has to be bundled somehow to take action. I am investigating, how to do so, but maybe anyone here with more knowledge of this topic can help faster.
Locally, it works for me.

[micdahl]

Hi again @Malian . I found out, how to get it running! From your phoenix app's folder:

cd deps/phoenix_live_view
mix esbuild.install
mix esbuild module
mix esbuild main
mix esbuild cdn
mix esbuild cdn_min

After that you can start the phx.server and see the magic happen! If I understand it right, the build step is done by the maintainers of this repository if the pull request is accepted. So this is only neccessary until then.

[josevalim]

Thank you for the PR, @micdahl! I believe we don't want to remove them either, because that would rebuild all phx-hook entries inside. So we need a way to force a new ordering without deleting.

[micdahl]

Hi @josevalim and thank you for the feedback. I have just started working with live view, so I am not into the details. I am just wondering why rebuilding would be a problem when reset is set to true. Isn't this the expected behavior of a reset?
Which other solutions do you have in mind? Maybe an additional flag to distinguish if the elements should be deleted during the reset (stream(socket, :elements, elements, reset: true, reset_delete: true))? Or a reordering of the elements inside of the dom via js?

[SteffenDE]

I can confirm that this fixes #2826 for me as well. The problem seems to have been introduced by beeed2e for issue #2657.

[nathanalderson]

Does not seem to fix #2816 (which is about the test client).

[Gigitsu]

We can apply the right sorting by explicitly setting the at attribute replacing this case block with the following:

if opts[:reset] do
  new_socket = update_stream(socket, name, &LiveStream.reset(&1))

  items
  |> Enum.with_index()
  |> Enum.reduce(new_socket, fn {item, i}, acc -> stream_insert(acc, name, item, Keyword.put(opts, :at, i)) end)
else
  Enum.reduce(items, socket, fn item, acc -> stream_insert(acc, name, item, opts) end)
end

or alternatively

if opts[:reset] do
  new_socket = update_stream(socket, name, &LiveStream.reset(&1))

  items
  |> Enum.reverse()
  |> Enum.reduce(new_socket, fn item, acc -> stream_insert(acc, name, item, Keyword.put(opts, :at, 0)) end)
else
  Enum.reduce(items, socket, fn item, acc -> stream_insert(acc, name, item, opts) end)
end

I don't know if it is preferable to use Keyword.put_new instead of Keyword.put to avoid overwriting the user provided at option but, IMO, there is no reason to provide the at parameter when resetting the stream.

I tested both locally and they work, I would be happy to open a PR for this.

Edit:
I made some mistakes in my previous example, it is necessary to replace the entire case block and not only a line of code.