Conflicts with TMail #796

Closed
thbar opened this Issue Feb 9, 2012 · 8 comments

Projects

None yet

4 participants

@thbar

TMail unfortunately defines a top-level StringInput class; this chokes with the input builder detection mechanism.

I know TMail is getting less and less used but in case someone else wonders, I left the issue here.

I don't know if it requires fixing or not - I will try to work-around that (using ActiveAdmin here).

Here's how to detect that (with REE 1.8.7 here):

> StringInput.method(:new).__file__
"/Users/thbar/.rvm/gems/ree-1.8.7-2011.03@app/gems/tmail-1.2.7.1/lib/tmail/stringio.rb"
``
@justinfrench

@thbar did you find a work-around? I think Rails dropped TMail over Mail, so feel like something that doesn't need to be "fixed" going forward. Closing for now, but please reopen if you'd still like to see something change in Formtastic.

@thbar

@justinfrench my client was using TMail for email receiving processing despite being in a Rails 3 app (for historical reasons).

In the end the work-around was this:

gem "tmail", "1.2.7.1", :require => false

then require only from the daemon that does the job.

Apart from a quick warning on the README or in the dependencies checker maybe, I don't think much change is needed in Formtastic!

@justinfrench

Thanks @thbar

@craigambrose

I've hit the same problem, and it wasn't possible for me to avoid requiring tmail in the web app. It seems pretty strange that formtastic defines StringInput safely inside a namespace but then checks for a StringInput class in the global namespace first. I guess the idea here is that people can write custom formtastic inputs in the global namespace, but I think it would be better if a namespace was defined for that. I've fixed the problem for myself by monkey patching formtastic as follows:

module Formtastic
  module Helpers
    module InputHelper
      protected
        # prevent exceptions in production environment for better performance
        def input_class_with_const_defined(as)
          input_class_name = custom_input_class_name(as)

          if Formtastic::Inputs.const_defined?(input_class_name)
            standard_input_class_name(as).constantize 
          else
            raise Formtastic::UnknownInputError, "Unable to find input class #{input_class_name}"
          end
        end

        # use auto-loading in development environment
        def input_class_by_trying(as)
          standard_input_class_name(as).constantize
        rescue NameError
          raise Formtastic::UnknownInputError, "Unable to find input class for #{as}"
        end        
    end
  end
end

Above, I'm no longer checking for custom inputs, and if I ever want to override them in this app, I'll need to do it in the formtastic namespace. I think the best possible solution to this problem would be to require people to put custom inputs into some namespace, any one, and then to call something like Formastic.register_custom_input_namespace('MyApp::Inputs') to add that to a list of namespaces that formastic looks for constants in. While many gems might declare classes with names like StringInput in the global namespace, since formtastic dynamically loads such classes by name, I think the responsibility for ensuring things stay out of global rests with formtastic.

@justinfrench justinfrench reopened this Aug 20, 2012
@justinfrench

@craigambrose Thanks for taking time to get to the bottom of this and articulate the problem. I agree this is an issue, and will work on this for the next release. Re-opening.

@did

hey guys, a dirt and quick patch in order to make formtastic work with namespaced custom inputs.
https://gist.github.com/4301317
It works fine within my application.
Best

@justinfrench

Long time since we've talked on this issue, but I still don't think this is squarely Formtastic's problem. It's great that @craigambrose and @did have patches above to help folks out in this situation.

Everything that Formtastic defines is in the Formtastic namespace. Developers have the option to define classes like StringInput in the global namespace because it's their application, much like they can define a Post, not an ActiveRecord::Post. It's their namespace, they can do whatever they want.

The root cause here is that unlike all the code in Formtastic and most modern libraries, TMail defines StringInput in the global namespace. An issue should be raised on TMail.

However, some blame does lie with Formtastic because we're depending on developers being able to use StringInput in the global namespace, and assuming that libraries like TMail aren't making that impossible.

So it does feel like we need custom inputs to (optionally) live in a namespace. For most apps, most of the time, what we have works. For apps relying on gems that polite the namespace, perhaps there needs to be some kind of configuration that says…

"Instead of looking for FooInput in app/inputs/foo_input.rb, look for Something::FooInput in app/inputs/something/foo_input.rb.

I think this provides the least disruptive path forward for most apps, but gives developers a way out when things go bad. If someone's interested in helping out, I'd love to see a patch. Happy to leave this open for now, but I won't have time to get this into 2.3.0 and/or 3.0.0 myself right now.

@justinfrench

This should be closed when #1043 is merged.

@justinfrench justinfrench added this to the 3.1 milestone Nov 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment