infinitered / ProMotion-menu

RubyMotion gem allowing you to easily setup a facebook or Path style hidden slide menu easily with the ProMotion gem.
74 stars 29 forks source link

app.detect_dependencies not always effective #38

Closed dunghopper closed 10 years ago

dunghopper commented 10 years ago

I am using ProMotion 2.0.1 and Promotion-menu 1.0.0.beta1, and RubyMotion 2.37

Despite setting app.detect_dependencies = true explicitly in my Rakefile (though I understand this is the default behavior, so that should be unnecessary), some ProMotion-menu files are still being loaded after my code in app, causing uninitialized constant errors.

Browsing through the source code, I noticed that insert_point is not actually used in ProMotion-menu.rb (extracted starting at line 9):

  core_lib = File.join(File.dirname(__FILE__), 'ProMotion')
  insert_point = app.files.find_index { |file| file =~ /^(?:\.\/)?app\// } || 0

  Dir.glob(File.join(File.dirname(__FILE__), 'ProMotion/menu/**/*.rb')).each do |file|
    app.files << file
  end

Changing this code as follows, to match more closely that in the ProMotion gem, works (at least in my case) to load all the files in the right order:

  core_lib = File.join(File.dirname(__FILE__), 'ProMotion/menu')
  insert_point = app.files.find_index { |file| file =~ /^(?:\.\/)?app\// } || 0

  Dir.glob(File.join(core_lib, '**/*.rb')).each.reverse do |file|
    app.files.insert(insert_point, file)
  end

It seems like this careful insertion into files should be unnecessary if detect_dependencies were doing its job, but I'm not clear on just how (or when) detect_dependencies does it's magic.

It's worth noting that the ProMotion gem also sets a bunch of files_dependencies explicitly, with the comment:

# For compatibility with libraries that don't use detect_dependencies. :-(

I don't know if something like that is also necessary here.

jamonholmgren commented 10 years ago

Awesome issue explanation. Seems like you've found the fix already. Would you like to submit a pull request and get the commit credit?

dunghopper commented 10 years ago

Yeah, I can make a pull request if you think this is the right fix.

Though I'm still curious why app.detect_dependencies = true doesn't seem to be working... any ideas?

jamonholmgren commented 10 years ago

It's not perfect by any means. It works best when you follow a very simple structure (e.g. limit nested module/classes, don't re-open classes, etc).

Some libraries actually turn detect_dependencies off (for example, motion_support) and that screws with things too.

It's been a thorn in my side for a long time. I like the dependency detector but there are certainly limitations that we have to work around.