hyper-mesh icon indicating copy to clipboard operation
hyper-mesh copied to clipboard

server_method macro looking for foo?= method

Open adamcreekroad opened this issue 8 years ago • 9 comments

I defined a method on my model:

server_method :past_due? do
  ...
end

Nothing is broken on the client, but I get this message:

Warning: Deprecated feature used in Job. Server side method past_due?=(true) must be defined using the 'server_method' macro.

It seems it's trying to call past_due?= in addition to past_due?.

Here's where it's happening (in Opal)

if (method['$==']("id")) {
} else {
  target.$send("" + (method) + "=", value.$first())
}

adamcreekroad avatar May 09 '17 19:05 adamcreekroad

I am thinking server_method is deprecated anyway in favor of using operations? I can't remember

catmando avatar May 22 '17 23:05 catmando

I'm also seeing this and it's pretty jarring. The remote_method does seem to work, there's just that warning. If I make foo= a server method then it stops working. Likewise if I define an attr_writer. So I can't find any way to get rid of the warning.

There may be a good argument for removing model server methods to encourage smaller model files and custom logic to go elsewhere. Plus having to add Hyperloop specific code to models feels a bit wrong.

But for my use case I can't see a better way:

  • I have a Message model where the body field stores markdown text.
  • I load a conversation on the client using Hyper Mesh AR relations, they're all batched into one nice request.
  • But I need the markdown as HTML to display (lets say I don't want to use a client side lib) so I add a server_method that converts the markdown. The benefit is it gets loaded in the same batch request.
  • I tried putting the markdown conversion in a ServerOp, but it was way more code and state etc, and every message triggered a seperate remote execute; they're not batched.

So advantage of server_method is it's automatically batched and done in the same request when loading a relation. As far as I can see there's no way to batch Operations other than making a parent operation that triages the data but that would be very complex and still a second request after the relation load.

sfcgeorge avatar Oct 19 '17 14:10 sfcgeorge

this apparently happens for any method that is not an attribute or somehow else defined. Client fetches by putting the method in the vector, lest's say the method is :past_due? When the data arrives, the value is assigned to the client RR "attribute/method/whatever", using '=', so you get: :past_due?=

so its maybe 2 bugs here.

  1. On reception of data client does not differentiate between attribute and server method properly
  2. method might be auto generated rails method, like :is_admin? for a boolean attribute :is_admin, value should then be applied to the attribute :is_admin using :is_admin= not :is_admin?=

I also find server_method very useful, its simple to use, little code to write, big value for development.

I will remove the deprecation message for lap12 and try to fix the behaviour, if nobody objects.

janbiedermann avatar Dec 14 '17 15:12 janbiedermann

if fixed the behaviour as follow: method_missing in hyper-mesh/lib/reactive_record/active_record/instance_methods.rb

the ! is cared for as it was.

1. Server Method

If the method is a server_method its name is left alone and values are set with name= and no warning is printed. example:

server_method :past_due?
stored name in server_methods and RR attributes: 'past_due?'
value set by RR with: past_due?=

2. Attribute

if the method is a attribute, its more AR like now, for booleans that is:

attribute: :is_admin
AR auto generated method: is_admin?
stored name in attributes and public_columns_hash: 'is_admin'
value fetched with method is_admin?: is_admin
value set by RR with: is_admin=

3. Not Attribute, Not Server method

deprecation_warning is printed to console method is called just with its name, whatever the result will be and

the method_missing got a little longer, but i removed the regexp so performance is still good enough, this method_missing is not hit so often.

https://github.com/ruby-hyperloop/hyper-mesh/blob/9b5dcf14a7eefc4fcd4921c3a5a95597d5ac0b8f/lib/reactive_record/active_record/instance_methods.rb#L88

will be done in lap12, ill close this

janbiedermann avatar Dec 14 '17 20:12 janbiedermann

I'm using a server method like an attribute in order to do some additional processing on a field but still have Hyper Mesh automatically update the frontend. This is what I did to get the warning to go away:

# model
class Post < ApplicationRecord
  # attribute :text

  server_method(:text_data) { text }
  
  if RUBY_ENGINE != "opal"
    def text_data=(data)
      self.text = Base64.decode64(data)
    end
  end
end

# component
record = Post.first
record[:text_data] = "14d5a2ff9"
record.save.then do
  # do stuff
end

You may wonder why, I'm actually using this method to do Carrierwave file uploads. Yep.

sfcgeorge avatar Mar 02 '18 14:03 sfcgeorge

@sfcgeorge so my understanding is that in the above all the server_method does is get the warning to go away in a very artificial manner. Is that correct?

if so it would seem that we just want to make server_method smart enough to understand what server_method("text_data=") { |data| self.text = Base64.decode64(data) } means.

catmando avatar Mar 03 '18 18:03 catmando

@catmando Yeah it is just to get rid of the error. Without suppressing the warning a million lines of base64 image data get printed to the browser console and Chrome freezes.

What I'm doing seems a bit hacky, but at the same time quite elegant because you get all the automaticity of hyper-mesh; validation errors; state updating; etc, and it's pretty simple but works great.

Yes quite possibly, I tried server_method("text_data=") and other variations but that didn't work right.

sfcgeorge avatar Mar 03 '18 19:03 sfcgeorge

@janbiedermann in your redo of the AR method_missing code, you automatically treat assignment to a server method as writing to an attribute. Any reason why?

My proposal is to allow server_methods to end in = and if they do then it will

  1. define the foo= method on the server per the supplied block
  2. write to a hidden attribute foo on the client, which will get sent to the server on save.

I'm not 100% happy with this, but...

catmando avatar Mar 04 '18 23:03 catmando

When i interpret my above comments correctly, to answer the why @catmando , then probably because it was like that and value set by RR with = hints that somewhere in RR code server_methods are treated as attributes and the value is assigned with = on the client. That it was like that and that it was the original intention of treating server_methods as attributes is also suggested by the opening post of this issue.

janbiedermann avatar Mar 05 '18 03:03 janbiedermann