search

leastbad / optimism

The missing drop-in solution for realtime remote form validation in Rails.
MIT License
361 stars 42 forks source link

restructuring syntax for rails 6.1 compatibility #24

Open fneves opened 4 years ago

fneves commented 4 years ago

Rails 6.1 is introducing a check to verify if the class is the class is not a Singleton, and if it is raises a TypeError. This PR simply changes the structure of the code and instead of calling mattr_accessor in the singleton class, simply calls it on the module. Making it ready for 6.1 or whoever is using the edge version of Rails.

leastbad commented 4 years ago

Thanks for bringing this to my attention! Can you explain why this check is performed? Why hate on singletons?

I'm actually very much hoping that Optimism won't be necessary on Rails 6.1, as DHH has said several times that remote form validations are "fixed" without providing details. All I know is that UJS is apparently going away and there is supposedly a better solution coming. Do you know anything about this?

fneves commented 4 years ago

Hi there. I am not a Rails team member and unfortunately I don't know exactly why this check was being added although I suppose it might have something to do with possible issues around multithreading.

I have read for a very long time about the new Turbolinks6 and the new shiny JS improvements, however I haven't really seen anything popping out, and for now I just like to use your gem :) Since I needed the activestorage proxy feature added to the edge rails version, I am using the edge version now. And this was making it incompatible. Since the change is simple and not harming the code, I simply decided to open a PR and solve this issue for anyone experiencing the same problem I have.

leastbad notifications@github.com escreveu no dia terça, 6/10/2020 à(s) 09:48:

Thanks for bringing this to my attention! Can you explain why this check is performed? Why hate on singletons?

I'm actually very much hoping that Optimism won't be necessary on Rails 6.1, as DHH has said several times that remote form validations are "fixed" without providing details. All I know is that UJS is apparently going away and there is supposedly a better solution coming. Do you know anything about this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/leastbad/optimism/pull/24#issuecomment-704126084, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE6AV7V3UDGSNLB6XUIYCDSJLKXHANCNFSM4SFTHFXQ .

leastbad commented 4 years ago

For sure, and I really appreciate it.

I will try out the PR on my local machine and make sure this doesn't break.

leastbad commented 4 years ago

Hi @fneves, just wanted to give you an update.

Unfortunately, the solution you've offered fails if you're also using StimulusReflex, which makes this patch a non-starter. It effectively over-writes the channel SR uses with "OptimismChannel" ... which is bad.

However, we want to get this working so we're looking at other ways we could change this that won't break API compatibility. For example, it might benefit from actually making Optimism a real Singleton. I'm going to be testing various options today.

fneves commented 4 years ago

did you manage to get any solution? I can help with something if you need to. How did you test with StimulusReflex (I honestly don't use it so it never crossed my mind to test with it)?

leastbad commented 4 years ago

Sorry for the delay, this is a volunteer project so unfortunately it can sometimes take time for change to occur.

I have to figure out how to install edge Rails on my system in a way that lets me spin up new projects. I know it's probably basic, but I just haven't done it, yet.

As for StimulusReflex, installing it is very easy. Please give it a go: https://docs.stimulusreflex.com/setup