stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Web Components Breaking In Stimulus Reflex Morph #511

Closed christopheragnus closed 3 years ago

christopheragnus commented 3 years ago

Bug Report

Describe the bug

Hey, I have encountered a bug in my view. Whenever I try to morph a partial containing Ionic Framework Web Components, the component within the morph tags is blank.

To Reproduce

So, I want the current list of alerts to be updated from the DB when a new alert has been successfully created in the Rails controller.

index.html.erb

<head>
    <script type="module" src="https://cdn.jsdelivr.net/npm/@ionic/core/dist/ionic/ionic.esm.js"></script>
    <script nomodule src="https://cdn.jsdelivr.net/npm/@ionic/core/dist/ionic/ionic.js"></script>
</head>
<body>
  <ion-card>
    <ion-card-header>
      <ion-card-title>Current Alerts</ion-card-title>
      <ion-card-subtitle>Swipe left to delete</ion-card-subtitle>
    </ion-card-header>
    <ion-list>
      <ion-item>
        <ion-card-subtitle slot="end">Active?</ion-card-subtitle>
      </ion-item>
      <div id="alert-list">
        <%= render partial: "alerts_list", locals: { price_targets: @price_targets }  %>
      </div>
    </ion-list>
  </ion-card>
</body>

_alerts_list.html.erb

  <% price_targets.each do  |alert| %>
    <ion-item-sliding>
      <ion-item>
        <ion-toggle data-reflex="click->Alerts#toggle_alert"
            data-id=<%= alert.id %>
            <%= alert.enabled ? "checked" : nil %> slot="end"
            ></ion-toggle>
        <ion-label>
          <h2><%= alert.ticker %></h2>
          <h3></h3>
        </ion-label>
      </ion-item>

      <ion-item-options side="end">
        <ion-item-option data-reflex="click->Alerts#delete_alert"
          data-id=<%= alert.id %>
          data-symbol=<%= alert.ticker %>
          >Delete</ion-item-option>
      </ion-item-options>
    </ion-item-sliding>
  <% end %>

price_targets_controller.erb

cable_ready["PriceTargetsChannel"].morph({
                                                 selector: "#alert-list",
                                                 html:          PriceTargetsController.render(
                                                   partial: "dashboard/alerts_list",
                                                   locals: { price_targets: @price_targets }
                                                 ),
                                                 children_only: true
                                               })

Expected behavior

The list of alerts should be updated with the morph.

Screenshots or reproduction

What it should look like: https://i.imgur.com/BDdu5R8.png

After the morph: https://i.imgur.com/G6JEK6l.png

Versions

StimulusReflex

External tools

Browser

leastbad commented 3 years ago

I'm not familiar with Ionic, but it sounds like it might be modifying the DOM after the page loads. This conflicts with the morphdom library's primary mechanism, which changes the state of the page to reflect what the server thinks it should look like. This all lines up with the main goal of StimulusReflex, which is to maintain all state on the server.

You have a few options:

HTH!

marcoroth commented 3 years ago

Hey @christopheragnus, thank you for reporting this issue.

We had a similar issue with Alpine some time ago in #329.

Alpine provides a function which reinitializes all components which are not initialized:

Alpine.discoverUninitializedComponents(function(el){ Alpine.initializeComponent(el) })

If something similar exists in Ionic you could call that function in a client-side afterReflex callback in your Stimulus ApplicationController to reinitialize the ionic components.

There is not much else to say other than what @leastbad suggested in his reply.

The issue has to do we morphdom, see in https://github.com/ionic-team/ionic-framework/issues/22821 and https://github.com/patrick-steele-idem/morphdom/issues/127.

I fear that we can't "solve" the underlaying issue in StimulusReflex itself.

leastbad commented 3 years ago

If Ionic does have something similar to Alpine's discoverUnitializedComponents method, you can find documentation on how to set up CableReady to accomodate Ionic here: https://cableready.stimulusreflex.com/customization#shouldmorph-callbacks

leastbad commented 3 years ago

Hi @christopheragnus, could you let us know how you're making out with this issue?

Our first priority is to make sure that you got your application working. Our second priority is to close inactive issues. ;)

christopheragnus commented 3 years ago

Hi @christopheragnus, could you let us know how you're making out with this issue?

Our first priority is to make sure that you got your application working. Our second priority is to close inactive issues. ;)

Hey there, I managed to fix it by using .replace() like below. It sees to reinitialize Ionic when I do this. Its still happening if I use morph().

cable_ready["PriceTargetsChannel"].replace({
                                                   selector: "#alert-list",
                                                   html:     render(
                                                     partial: "dashboard/alerts_list",
                                                     locals: { price_targets: @price_targets }
                                                   )
                                                 })
leastbad commented 3 years ago

I'm glad that you got it working, but a complete replacement shouldn't be necessary. Reinitializing should be a last resort.

Were you able to find the equivalent of Alpine's discoverUnitializedComponents method? That would help us help other Ionic users in the future.

christopheragnus commented 3 years ago

I double-checked the Ionic Framework APIs but there doesn't seem to be a fix similar to Alpine's discoverUnitializedComponents method. I took a look here: https://github.com/ionic-team/ionic-framework/issues/22821

<ion-card class="md hydrated">
  a<div data-controller="indexcharts" data-indexcharts-chartdata="<%= @index_chartdata %>">
    <%= line_chart @index_chartdata, id: "index-chart", points: true, loading: "Loading...", library: {
      plugins: {
                 legend: {
                 disply: false
                 },
                 tooltip: {
                 mode: 'index',
                 intersect: false
                 }
      },
      hover: {
             mode: 'nearest',
             intersect: false
                },
    } %>
    <button data-reflex="click->Indexcharts#set_data_source" data-reflex-dataset="combined">Test</button>
  </div>
  </ion-card>

My current workaround is adding class="hydrated" if it is just doing a morph - but it doesn't work if for the above example so I need to do a replace().

leastbad commented 3 years ago

Well, that's a shame... but hey - you got it working! That's awesome.

I'm going to close this issue, but definitely let us know if you ever find a way to make things easier for Ionic users.

leastbad commented 2 years ago

Hey @christopheragnus, I wanted to provide an update because I think we saw something similar happen with Shoelace web components over the past few days. @ParamagicDev brought it up and the "solution" was assessed on Discord: https://discord.com/channels/629472241427415060/891395933089189918/952747035491188786

The problem is that many web components modify their outerHTML (via changing attributes, such as class) as part of their initialization process. That means that the markup you are rendering on the server might get modified by the Ionic component once it hits your page.

In the Shoelace scenario, we have a helper that writes out

<sl-button-group>
        <sl-button type="submit" name="commit">Sign up</sl-button>
      </sl-button-group>

but I saw that the actual outerHTML of the button was

<sl-button-group>
        <sl-button type="submit" name="commit" variant="default" size="medium" class="sl-button-group__button sl-button-group__button--first sl-button-group__button--last">Sign up</sl-button>
      </sl-button-group>

To "fix" it, I modified the helper call so that it had the same attributes it would gain after initialization. In other words, this

<%= sl_submit {t("auth.signup")} %>

became

<%= sl_submit(variant: "default", size: "medium", class: "sl-button-group__button sl-button-group__button--first sl-button-group__button--last") {t("auth.signup")} %>

Now, you might not love the extra effort or verbosity, but it is a practical, low-tech solution that doesn't require anything beyond access to the Inspector and some patience/willingness to iterate. Unfortunately, so long as web component authors insist on mutating their outerHTML, morphdom is going to need to be given HTML fragments that match the true, post-initialization state of the components.