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

Remove target children with duplicate ID before append/prepend #240

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Remove target children with duplicate ID before append/prepend #240

merged 1 commit into from
Jun 8, 2021

Conversation

jeromecornet
Copy link
Contributor

Fixes #132

@dhh this implements Option C of #132 (comment)

When appending/prepending, children of the template may contain the same ID as children of the existing target.
(this is common when using turbo-rails or other frameworks where the append/prepend action comes both as a result of the response to the original action and as a broadcasted action from a background job).

When this is the case, the generated result contains duplicate children ID which is undesirable.

This PR changes the behaviour of the append/prepend actions so that the existing children of the target which match the children of the template are removed before appending the template.

Assuming the following contrived example:
Target:

<ul id="vehicles">
  <li id="vehicle_1">Old car</li>
  <li id="vehicle_2">New car</li>
</ul>

Stream element:

<turbo-stream action="append" target="vehicles">
  <template>
      <li id="vehicle_1">Old car with updates</li>
      <li id="vehicle_3">Newer car</li>
  </template>
</turbo-stream>

With the current code, the result is:

<ul id="vehicles">
  <li id="vehicle_1">Old car</li>
  <li id="vehicle_2">New car</li>
  <li id="vehicle_1">Old car with updates</li>
  <li id="vehicle_3">Newer car</li>
</ul>

This PR changes it so the result is

<ul id="vehicles">
  <li id="vehicle_2">New car</li>
  <li id="vehicle_1">Old car with updates</li>
  <li id="vehicle_3">Newer car</li>
</ul>

@acushlakoncept
Copy link

I am currently having this issue, how do I apply the solution in my code as the PR is yet to be merged?

@jeromecornet
Copy link
Contributor Author

jeromecornet commented May 24, 2021

I am currently having this issue, how do I apply the solution in my code as the PR is yet to be merged?

@acushlakoncept if you are using turbo-rails with sprockets, I have created a temporary fork with a patched turbo.js until this gets merged.
gem "turbo-rails", github: "jeromecornet/turbo-rails", branch: "dedup"

@acushlakoncept
Copy link

Thank you @jeromecornet, is gem "turbo-rails" ... different from gem 'hotwire-rails' that I had?
Sorry I am new with turbo stuff

@jeromecornet
Copy link
Contributor Author

@acushlakoncept The gem hotwire-rails depends on the other gem turbo-rails, so if you only have hotwire-rails in your Gemfile, add another line as mentioned above to force the version of turbo-rails, until both turbo and turbo-rails get updated with that fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate content, Turbo stream added an already existing item
3 participants