Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly require dependencies to avoid ordering issues #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickbrowne
Copy link

I ran into an issue attempting to use the master version (which fixes a comparison bug we were having in 0.6.0) where the order of files being loaded in lib/equivalent-xml.rb was inconsistent. This leads to an issue when requiring the gem:

Bundler::GemRequireError: There was an error while trying to load the gem 'equivalent-xml'.
Gem Load Error is: uninitialized constant EquivalentXml::Proxy::Base

Dir[File.expand_path('../equivalent-xml/proxy/*',__FILE__)] seems to return an array of paths in an unintended order. This causes the subclasses to load before the super class, which results
in EquivalentXml::Proxy::Base being undefined.

On my machine it will load in the order:

equivalent-xml/lib/equivalent-xml/proxy/oga.rb
equivalent-xml/lib/equivalent-xml/proxy/nokogiri.rb
equivalent-xml/lib/equivalent-xml/proxy/base.rb

I've changed the requires to be more explicit, each subclass now requires the base class, and lib/equivalent-xml.rb requires each of the subclasses. It's possible you're not keen on this approach, but I thought I would propose it to see what you thought.

`Dir[File.expand_path('../equivalent-xml/proxy/*',__FILE__)]` seems
to return an array of paths in an unreliable manner. Occasionally
causing the subclasses to load before the super class, which results
in EquivalentXml::Proxy::Base being undefined.
@nickbrowne
Copy link
Author

I'm not sure why those builds are failing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant