-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Remove ActiveSupport::Dependencies.reference
#5357
Remove ActiveSupport::Dependencies.reference
#5357
Conversation
This was deleted from Rails: rails/rails@14d4edd As far as I can tell, it was meant to add a performance boost at some point in the past but doesn't seem to do anything useful these days. cc @fxn
This is an internal class which is no longer needed.
5696295
to
99d08e6
Compare
So that this branch can be used with rails/main.
Thanks @ghiculescu ❤️. |
So that this branch can be used with rails/main.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello!!! Thanks for submitting this patch 🎉 I just ran into this trying to set up Devise on a new Rails app and wanted to leave a review with the results of my testing this branch.
I'm running:
- Rails 7.0.0.alpha
- Ruby 3.0.0
- Devise 4.8.0
Before this branch, when I attempted to run rails generate devise:install
after adding Devise to the Gemfile and bundling the rake task failed with the error
/Users/myuser/myapp/vendor/bundle/ruby/3.0.0/gems/devise-4.8.0/lib/devise.rb:321:in `ref': undefined method `reference' for ActiveSupport::Dependencies:Module (NoMethodError)
I pointed my version of devise in the gemfile to this branch gem "devise", github: "ghiculescu/devise", branch: "patch-2"
, and repeated bundling and then trying to run rails generate devise:install
. This time it installed successfully and I did not see any exceptions.
What's the policy on merging these kinds of things? I am spinning up a new project and wanted to just start on rails 7 since the project won't launch for a bit anyway. |
@mattrasband The policy is basically that I haven't been able to look into anything here recently 🙈 (personal and work stuff), but I should have some time in the coming days and will take a better look. In the meantime feel free to use this PR branch directly. @ghiculescu appreciate the work on this one. |
No worries @carlosantoniodasilva - thanks for the great work! |
Hey folks, thanks for being on top of this. Just want to add a data point that I encountered the same issue today. I will keep an eye out on this thread for now but I really appreciate all of your time here. |
So that this branch can be used with rails/main.
Anything we can do to help this along? |
So that this branch can be used with rails/main.
Thank you for your work on this. Still works with Rails |
So that this branch can be used with rails/main.
I'm merging this to a separate branch for the time being, and will get it into master soon. Thanks @ghiculescu and everyone else here. |
This is now on master, thanks again. Side note: there's one warning triggered by the tests that I have to look into, I think it's Rails related, I'll circle back on it. |
Will there be a new stable release for this? I was trying the Rails 7 alpha and ran into this :) |
With Rails 7.0.0 final out, would it be possible to get a devise release that has this in it? |
v4.8.1 was released just this morning. 43800b4 |
Thanks @carlosantoniodasilva 🙏 |
So that this branch can be used with rails/main.
The `ActiveSupport::Dependencies::Reference` class was removed from Rails in version 7 [1] but was still used by Devise 4.7.3. This was fixed in Devise [2], so that gem now needs updating to allow us to upgrade to Rails 7. 1: rails/rails@14d4edd 2: heartcombo/devise#5357
This was deleted from Rails: rails/rails@14d4edd, which means Devise currently crashes when run against Rails main.
As far as I can tell, it was meant to add a performance boost at some point in the past but doesn't seem to do anything useful these days.
I also updated CI to run against the Rails
main
branch.cc @fxn