joshleblanc / view_component_reflex

Call component methods right from your markup
http://view-component-reflex-expo.grep.sh/
MIT License
291 stars 27 forks source link

Components are not keeping state #43

Closed cj closed 3 years ago

cj commented 3 years ago

Currently the component is not keeping state which is giving me this error ERROR: undefined method+' for nil:NilClass`.

I have uploaded a demo repo here https://github.com/cj/app-template and the component I am testing is https://github.com/cj/app-template/tree/main/app/components/form_component.

I'm sure I am just missing something!

cj commented 3 years ago

Ok - so it turns out that was caused to a miss-configuration which I fixed and it is not triggering. There are still some issues however, I am getting all these warnings and a couple of error messages:

CleanShot 2020-12-15 at 16 04 22

I also noticed that changing the handle click to:

    def handle_click
      @clicked = true
      @count += 1

      refresh!("testing-clicked", selector)
    end

Refreshes the entire components dom and not just what is wrapped in <div data-key="testing-clicked">

joshleblanc commented 3 years ago

Typically you need to add an id to the element triggering the reflex for SR to find it again.

joshleblanc commented 3 years ago

refresh("testing-clicked", selector) would try to update the tag testing-clicked, not data-key="testing-clicked".

The selector parameter is the unique selector for the component. So it would refresh the entire component, like you noticed.

cj commented 3 years ago

@joshleblanc It seems to work by either doing refresh!("testing-clicked", selector) and <div data-key="testing-clicked"> OR refresh!("#testing-clicked", selector) and <div id="testing-clicked"> . The issue is not that it isn't getting triggered, but that it still refresh the entire component.

The errors and warnings from above are without refresh! all together.

joshleblanc commented 3 years ago

Not sure about the error. I'll have to look into that more.

cj commented 3 years ago

@joshleblanc ah that makes sense about the selector, the issue now is count does not get updated....

joshleblanc commented 3 years ago

If you turn off debug mode in SR's frontend, does it work?

cj commented 3 years ago

CleanShot 2020-12-15 at 16 13 02

cj commented 3 years ago

with debug off I still have the same issue. I have updated the repo here https://github.com/cj/app-template

joshleblanc commented 3 years ago

In your repo as it exists, you're not rendering the component again at all.

    def handle_click
      @clicked = true
      @count += 1

      # refresh implicitly prevents the default rendering of the component. 
      # With this call you are *only* refreshing the #testing-clicked element
      refresh!("#testing-clicked")
    end

Testing without the refresh! call, and refresh!("#testing-clicked", selector) both worked as expected for me.

qic04DAJI0

cj commented 3 years ago

Ah, I needed to move down the <%= @count %> inside the div, I totally missed that! THANK YOU!

Were you getting the errors and warning in the console?

joshleblanc commented 3 years ago

Yeah, I can reproduce those. Going to look into it more

cj commented 3 years ago

@joshleblanc cheers!

cj commented 3 years ago

Let me know if you want me to test anything or need some help.

joshleblanc commented 3 years ago

Will do

cj commented 3 years ago

@joshleblanc Hope you're having a great weekend!

I hope I am not going crazy (or doing something wrong), but I think I ran into an actual issue with state and models.

I have updated https://github.com/cj/app-template, and the files in the example below are https://github.com/cj/app-template/tree/main/app/components/form_component.

The issue is the @user.email update is not being reflected in the dom.

CleanShot 2020-12-19 at 11 28 16

joshleblanc commented 3 years ago

Ah, this is a case where the documentation could take some work.

When a component is initialized, we compare the stored state with the initial state of the component (Usually passed in via parameters). In this case, when comparing User.new to the old User.new, the system will always think it's receiving a new value, and replace the one from state.

So to fix this, we just need to define the permit_parameters method to persist the @user variable.

def permit_parameter?(initial_param, new_param)
  if new_param.is_a? User
    false
  else
    super
  end
end 
cj commented 3 years ago

@joshleblanc thank you, I thought I was going crazy!

I ended up adding this to my base component which works well:

    def permit_parameter?(_initial_param, new_param)
      if new_param.is_a?(ActiveRecord::Base)
        false
      else
        super
      end
    end

I have noticed now there are oddities when using inputs, it ranges from the div disappearing to a querySelector error which I get if I add debounced:. (I updated the app-template repo):

CleanShot 2020-12-19 at 14 46 28

cj commented 3 years ago
Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Document': '#2SoLt7V5m4Jr6OhR8eDFN' is not a valid selector.
    at findElement (http://localhost:3000/packs/application-81a61e25a0a880082712.js:32516:25)
    at HTMLDocument.<anonymous> (http://localhost:3000/packs/application-81a61e25a0a880082712.js:33425:82)
    at dispatch (http://localhost:3000/packs/application-81a61e25a0a880082712.js:17522:11)
    at Object.dispatchEvent (http://localhost:3000/packs/application-81a61e25a0a880082712.js:17630:5)
    at Object.perform (http://localhost:3000/packs/application-81a61e25a0a880082712.js:17836:88)
    at Subscription.received (http://localhost:3000/packs/application-81a61e25a0a880082712.js:33044:63)
    at http://localhost:3000/packs/application-81a61e25a0a880082712.js:6591:94
    at Array.map (<anonymous>)
    at Subscriptions.notify (http://localhost:3000/packs/application-81a61e25a0a880082712.js:6590:28)
    at Connection.message (http://localhost:3000/packs/application-81a61e25a0a880082712.js:6424:37)

Is the other error I get sometimes.

cj commented 3 years ago

@joshleblanc I have also noticed if you remove https://github.com/cj/app-template/blob/main/app/components/form_component/base.rb#L41 this https://docs.stimulusreflex.com/patterns#autofocus-text-boxes trick no longer works.

joshleblanc commented 3 years ago

IDs can't start with a number