Refactor Cells Namespacing

[ghost]

when we integrate cells (https://github.com/apotonick/cells) we get namespace conflicts. Any suggestions here?

[robinboening]

I think the only way to get it working is to namespace that cells gem, which is currently not.

Do you have an example stacktrace with errors?

On 12.02.2014, at 11:47, Florian Schade notifications@github.com wrote:

when we integrate cells (https://github.com/apotonick/cells) we get namespace conflicts. Any suggestions here?

— Reply to this email directly or view it on GitHub.

[ghost]

it is namespaced... as Cells. if you change in

Alchemy::Page
include Cells

to

include Alchemy::Page::Cells

all works fine! tested this way

Alchemy::Page.class_eval do
  Object.send(:remove_const, :Cells)
  include Alchemy::Page::Cells
end

what i also did is overwrite the pages helper to get render_cell working

module Alchemy
  module PagesDecoratorHelper
    include Alchemy::PagesHelper

    def render_cell(name, state, *args, &block)
      ::Cell::Rails.render_cell_for(name, state, self, *args, &block)
    end

    def render_cms_cell(name, options={})
      default_options = {
          from_page: @page,
          locals: {}
      }
      options = default_options.merge(options)
      cell = options[:from_page].cells.find_by_name(name)
      return "" if cell.blank?
      render partial: "alchemy/cells/#{name}", locals: {cell: cell}.merge(options[:locals])
    end
  end
end

[tvdeyen]

Damn, wrong button :)

[tvdeyen]

What version are you using?

[tvdeyen]

It is totally ok, to proper namespace the includes, but I don't know if we really need to rename the helpers. If we do it, we would need to rename all helpers. I think of a proxy object on which one could call the helpers. I.e.: alchemy.render_cells

Ideas? @robinboening @masche842

[ghost]

sounds good for me. thx

[tvdeyen]

Ok, I guess this can be closed for now.

[robinboening]

Alchemy::Page.class_eval do
  Object.send(:remove_const, :Cells)
  include Alchemy::Page::Cells
end

This is like a shoot in the head. You can also do this - it also works:

Alchemy::Page.class_eval do
  require 'alchemy/page/cells'
  include Alchemy::Page::Cells
end

[tvdeyen]

This hack is not needed anymore: d8b9fac1abf8440b4c856f9005d385797d6e8147

The only issues is the render_cell helper. But this can be avoided with above mentioned fix from @fschade

[robinboening]

It is still needed. Without the hack your not including Alchemy::Page::Cells as you would expect. Instead you will get ::Cells included and receive a warning like warning: toplevel constant XX referenced by YY::XX". I think thats an issue from Ruby which is not fixed since 2003: https://groups.google.com/forum/#!searchin/comp.lang.ruby/warning$3A$20toplevel$20constant$20XX$20referenced$20by$20YY$3A$3AXX/comp.lang.ruby/E-uspv678CM/Wpj4d5SYdH8J

[tvdeyen]

Did you tested this? Because I think it should be working. Alternatively we could use include ::Alchemy::Page::Cells. This has to work.

[ChristianPeters]

I am using the 2.9.stable branch and have focused on making Alchemy work at all first. (Later, I have to make a cell from the Cells gem work inside an Alchemy page.)

What I did:

config/application.rb:

    config.to_prepare do
      Dir.glob(Rails.root.join("app/**/*_decorator*.rb")) do |c|
        Rails.configuration.cache_classes ? require(c) : load(c)
      end
    end

app/models/alchemy/page_decorator.rb:

Alchemy::Page.class_eval do
  require 'alchemy/page/cells'
  include Alchemy::Page::Cells
end

The problem is not completely solved with this workaround, as I am still experiencing exceptions like undefined method 'cells' for #<Alchemy::Page:0x007fd099d074a8>. They occur when changing e.g. the en.yml without restarting the server in development mode. While this is not a big show stopper, it is still annoying.

[ChristianPeters]

Also, I am experiencing these warnings that Robin mentioned:

• bundler/gems/alchemy_cms-dc2785a84ca5/app/models/alchemy/page.rb:71: warning: toplevel constant Cells referenced by Alchemy::Page::Cells
• /app/models/alchemy/page_decorator.rb:5: warning: toplevel constant Cells referenced by Alchemy::Page::Cells

[tvdeyen]

Ok, how do we solve these issues? Are they fixable? I want to release 2.9 soon, and if this is still an issue I would love to have a fix for it.

[robinboening]

I dont see a way to get rid of these warnings without moving the code from Page::Cells back into the Page model, or renaming it.

I'd guess the other issue with the undefined method exception @ChristianPeters mentioned would also be fixed then.

[tvdeyen]

And what if we would rename the Alchemy::Page::Cells module into something like CellsConcerns or similar. It is just an ActiveRecord::Concern to slim down the Page class. Not the best practice anyway. So the name is completely irrelevant here.

[robinboening]

Thats what I mean. We should try it. I think it should fix that.

[ChristianPeters]

You tackled the name conflict with the Cells constant but not the render_cell helper, right? I had to include @fschade's workaround to make my app work as before installing Alchemy. I guess it would be nice to namespace it, too: render_alchemy_cell

[tvdeyen]

I really have a problem with renaming the helper just for this "edge case" of combining these two gems. The workaround of @fschade is working and can be used by others using these gems together.

If we would rename these helpers in 2.9, we would need to deprecate them, etc, pp. These helpers are "sugar on the top". Rendering cells and elements is trivial. just load them and render a partial. And to be consistent, we would need to rename all "frontend" helpers, which would break a lot of sites.

So, sorry about, but we won't rename them now.

What do we do to solve the namespacing issues? @robinboening @ChristianPeters

[ChristianPeters]

I haven't found the time to look into the namespacing issues yet.

Regarding @fschade's workaround for render_cell: You can reduce monkey patching by aliasing Alchemy's helper:

app/helpers/alchemy/pages_helper_decorator.rb:

Alchemy::PagesHelper.module_eval do
  # Override Alchemy's render_cell helper by restoring the one from the Cells gem
  # See https://github.com/magiclabs/alchemy_cms/issues/462
  alias_method :render_alchemy_cell, :render_cell

  def render_cell(name, state, *args, &block)
    ::Cell::Rails.render_cell_for(name, state, self, *args, &block)
  end
end

[tvdeyen]

I like this approach. Maybe we should make Gist out of it, so people can find it via Google or we put it in the Wiki?

[robinboening]

@ChristianPeters I also thought about a method alias, but wanted to have tested it before mentioning it. :) But it should exactly do what we want. Have you already tested it in production?

[ChristianPeters]

Yes, it's works.

[tvdeyen]

Ok, I will close this then. We have a suitable workaround for this "issue".

[tvdeyen]

We backported 461308f20ed49b636b7a6ec21294307aae4b1045 into 2.9-stable so you do not need the hack for removing the Cells constant anymore.