Shouldn't Element be a Native? about opal-jquery HOT 16 CLOSED

Yes, it should. But there are problems we need to address first. The main one is the type of object to return from native calls (via method_missing). For example, Element#find would be partially broken. Native#method_missing would see the result as a native object, but it would get wrapped as a Native, not an Element.

Sure, we could just use a plain wrapper for Element, but we are also adding custom methods. Which means we either need to have some clever way to detect what class a native object should we wrapped as, or, we have to manually do lots of coercion.

We could introduce properties to natives to say what classes they could become. e.g., we could add something like this to $.fn:

$.fn.toOpal = function() {
  return Opal.Element.$new(this);
};

And Native, when encountering a native object, could simply check if the object can coerce itself into the opal world. If not, just return a simple Native instance.

Just a thought.

from opal-jquery.

As an example:

class Native
  def method_missing(*)
    %x{
      // all logic here

      if (typeof (result.toOpal) === "function") {
        return result.toOpal();
      }
      return #{ Native(`result`) };
    }
  end
end

from opal-jquery.

As @meh suggests, perhaps Element#method_missing is a better place for this. It will let us have a lot more control over what happens.

from opal-jquery.

I tried patching Element to add back method_missing, but it is currently broken causing Uncaught TypeError: Object [object Object] has no method '$test_method'.

I relied a lot on this feature for jquery plugins.

from opal-jquery.

Can you show me the code for the method_missing?

from opal-jquery.

I tried patching Element with the removed code ( 5e14389#L1L92 ):

class Element
  # Missing methods are assumed to be jquery plugins. These are called by
  # the given symbol name.
  def method_missing(symbol, *args, &block)
    %x{
      if (#{self}[#{symbol}]) {
        return #{self}[#{symbol}].apply(#{self}, args);
      }
    }

    super
  end
end

But even the following does not get called:

class Element
  def method_missing(symbol, *args, &block)
    p symbol
  end
end

Patching normal classes with method_missing works fine. Using opal v0.4.4.

from opal-jquery.

So you're saying method_missing doesn't get called when doing Element.find('bleh').stuff?

from opal-jquery.

Element is currently bridged to jquery instances, so it can't support method missing as we don't control it's prototype. We need to turn it into a wrapper class (Native) to get real method missing working.

from opal-jquery.

Yes, and not even with Element.new.test_method.

from opal-jquery.

@adambeynon oh, makes sense.

from opal-jquery.

@adambeynon Ok, thanks for the explanation. Would be nice to have Element#method_missing.

from opal-jquery.

As it turns out, trying to make Element act like a native is very tricky, and lots of awkward corner cases keep popping up. #on, #off, #find are particular pain points. For a short term solution, perhaps we should improve Element#expose which could manually add methods for these plugins you were using.

Element.expose :modal

Which would add an instance method for calling $().modal(). Its not as good as a method_missing solution, but Element needs a big rethink on how to handle wrapping jquery objects.

from opal-jquery.

For most of my current use cases the Element#expose is enough as it is now, so I can wait until progress is made on method_missing.

from opal-jquery.

As an update, I have (roughly) added native support to Element as per: 03dbe5d.

This commit basically means that instances of Element wrap an internal jquery object.

from opal-jquery.

So, turns out wrapping jQuery wasn't that great. Most methods needed to keep unwrapping/wrapping instances as before we just made the most out of toll-free bridging. For now, Element#expose is the way to go.

from opal-jquery.

👍

from opal-jquery.