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

fix: eject generator failing due to undefined source_location method #3566

Merged

Conversation

Eddayy
Copy link
Contributor

@Eddayy Eddayy commented Jan 3, 2025

Description

Ejecting components now fails with source_location undefined error, this is because it's used on the class directly while it's supposed to be used on methods
https://ruby-doc.org/3.3.6/Method.html#method-i-source_location

Fixes # (issue)
Will use const_source_location instead to find the class

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

  1. Step 1
  2. Step 2

Manual reviewer: please leave a comment with output from the test if that's the case.

@github-actions github-actions bot added the Fix label Jan 3, 2025
Copy link

codeclimate bot commented Jan 3, 2025

Code Climate has analyzed commit fc70f6d and detected 0 issues on this pull request.

View more on Code Climate.

@Eddayy Eddayy changed the title fix: reject generator failing due to undefined source_location fix: eject generator failing due to undefined source_location Jan 3, 2025
@Eddayy Eddayy changed the title fix: eject generator failing due to undefined source_location fix: eject generator failing due to undefined source_location method Jan 3, 2025
@Paul-Bob
Copy link
Contributor

Paul-Bob commented Jan 3, 2025

Hi @Eddayy, thanks for submitting this. Could you please specify which component failed to eject and provide the Rails/Ruby version used?

@Eddayy
Copy link
Contributor Author

Eddayy commented Jan 6, 2025

Hi @Eddayy, thanks for submitting this. Could you please specify which component failed to eject and provide the Rails/Ruby version used?

Sorry i didn't include it, here are the details
Ruby 3.3.5
Rails 7.1.2
Command used bin/rails g avo:eject --component Avo::Views::ResourceIndexComponent

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Jan 6, 2025

Thanks for sharing @Eddayy

It's not failing on my side, any tip on how to reproduce it?

╭─bob@machine ~/Projects/gems/avo/spec/dummy ‹main●› 
╰─$ ruby -v & rails -v & bin/rails g avo:eject --component Avo::Views::ResourceIndexComponent
[1] 17013
[2] 17014
ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
[1]  - 17013 done       ruby -v
Rails 7.1.2
[2]  + 17014 done       rails -v
warning: parser/current is loading parser/ruby33, which recognizes 3.3.6-compliant syntax, but you are running 3.3.5.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Running via Spring preloader in process 17107
Avo community 3.15.7
By ejecting the 'Avo::Views::ResourceIndexComponent' you'll take on the responsibility for maintain it.
Are you sure you want to eject the 'Avo::Views::ResourceIndexComponent'? [y/N] y
      create  app/components/avo/views/resource_index_component.rb
      create  app/components/avo/views/resource_index_component.html.erb

@Eddayy
Copy link
Contributor Author

Eddayy commented Jan 7, 2025

@Paul-Bob, turn out I'm having the issue because when installing avo on a new rails app, it install a newer view_component gem with version 3.21.0 instead of 3.20.0 used in the dummy app, 3.21.0 removes source_location and uses identifier instead, i'll updated it accordingly, it works for avo using both before and after the change

Related PR
ViewComponent/view_component#2153

Copy link
Contributor

@Paul-Bob Paul-Bob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution @Eddayy!

@Paul-Bob Paul-Bob merged commit 85cbb77 into avo-hq:main Jan 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants